script-server icon indicating copy to clipboard operation
script-server copied to clipboard

Walk runners dir

Open brunomgalmeida opened this issue 4 years ago • 6 comments

This provides a better way to organize things when we have many scripts and runners

brunomgalmeida avatar Mar 24 '21 13:03 brunomgalmeida

Hi @brunomgalmeida thanks for the PR! I think this is a nice feature

Would you mind adding a test for it, please? In tests.config_service_test.ConfigServiceTest

bugy avatar Mar 24 '21 15:03 bugy

thanks @bugy I am having some difficulty understanding the tests parts.

Would you be able to help me writing the test for this one, and I should be able to write tests from here onwards.

How can I run the tests?

brunomgalmeida avatar Mar 30 '21 11:03 brunomgalmeida

Hi @brunomgalmeida you could run the tests from src folder. The command is: python3 -m unittest discover -s tests -p "*.py" -t . It will run all the tests. I believe by replacing "*.py" with a specific filename you can run only a single file

Regarding the test, you might copy test_list_configs_when_multiple test method and adapt it to your changes:

    def test_list_configs_when_multiple_and_subfolders(self):
        _create_script_config_file('conf_x', subfolder = 's1')
        _create_script_config_file('conf_y', subfolder = 's2')
        _create_script_config_file('A B C', subfolder = os.path.join('s1', 'inner'))

        configs = self.config_service.list_configs(self.user)
        conf_names = [config.name for config in configs]
        self.assertCountEqual(['conf_x', 'conf_y', 'A B C'], conf_names)

bugy avatar Mar 30 '21 11:03 bugy

  1. The problem with using
files.append(os.path.join(_root, name))

is that the path returned includes the config+runners directory

FileNotFoundError: [Errno 2] No such file or directory: '../testconf/testconf/runners/Script2/Script2.json'

Hence the funky regular expression to remove it. If your only concern is the Windows paths we can update the regex to

files.append( re.sub ( r'^.*(/|\\)runners(/|\\)', '', os.path.join( _root, name) ) )
  1. "PS could you change the formatting rules please" Noted

brunomgalmeida avatar Jul 20 '21 15:07 brunomgalmeida

Let me check it later today

bugy avatar Jul 21 '21 08:07 bugy

I changed the method this way:

    def _visit_script_configs(self, visitor):
        configs_dir = self._script_configs_folder

        files=[]
        # Read config file from within directories too
        for _root, _dirs, _files in os.walk(configs_dir, topdown=True):
            for name in _files:
                files.append(os.path.join(_root, name))

        configs = [file for file in files if file.lower().endswith(".json")]

        result = []

        for path in configs:
            try:
                content = file_utils.read_file(path)

                visit_result = visitor(path, content)
                if visit_result is not None:
                    result.append(visit_result)

            except StopIteration as e:
                if e.value is not None:
                    result.append(e.value)

            except:
                LOGGER.exception("Couldn't read the file: " + path)

        return result

For me both tests and real server runtime works. For the following structure: image

bugy avatar Jul 24 '21 08:07 bugy

Merged to master

bugy avatar Mar 11 '23 12:03 bugy