From d9a3ddcb4533031784295a23f54dcbb105cf89cc Mon Sep 17 00:00:00 2001 From: Hakkyu Kim Date: Thu, 16 Dec 2021 15:59:16 +0900 Subject: [PATCH] Fix --run-on-changed-packages option (#303) * Rename symbols * Rewrite get_target_plugins * Add tests * Relocate files --- {tools => .github}/recipe.yaml | 0 .github/workflows/integration_test.yml | 4 +- tools/commands/build_example.py | 4 +- tools/commands/command_utils.py | 71 +++++++++++--------- tools/commands/integration_test.py | 4 +- tools/{commands => }/requirements.txt | 0 tools/tests/__init__.py | 0 tools/tests/test_command_utils.py | 90 ++++++++++++++++++++++++++ 8 files changed, 137 insertions(+), 36 deletions(-) rename {tools => .github}/recipe.yaml (100%) rename tools/{commands => }/requirements.txt (100%) create mode 100644 tools/tests/__init__.py create mode 100644 tools/tests/test_command_utils.py diff --git a/tools/recipe.yaml b/.github/recipe.yaml similarity index 100% rename from tools/recipe.yaml rename to .github/recipe.yaml diff --git a/.github/workflows/integration_test.yml b/.github/workflows/integration_test.yml index 88a32792e..97e3d3e55 100644 --- a/.github/workflows/integration_test.yml +++ b/.github/workflows/integration_test.yml @@ -25,7 +25,7 @@ jobs: run: | export PATH=`pwd`/flutter-tizen/bin:$PATH ./tools/run_command.py test \ - --recipe ./tools/recipe.yaml \ + --recipe ./.github/recipe.yaml \ --generate-emulators \ --run-on-changed-packages \ --base-sha=$(git rev-parse HEAD^) @@ -34,5 +34,5 @@ jobs: run: | export PATH=`pwd`/flutter-tizen/bin:$PATH ./tools/run_command.py test \ - --recipe ./tools/recipe.yaml \ + --recipe ./.github/recipe.yaml \ --generate-emulators diff --git a/tools/commands/build_example.py b/tools/commands/build_example.py index 5b79b3f97..c81cbd165 100755 --- a/tools/commands/build_example.py +++ b/tools/commands/build_example.py @@ -33,8 +33,8 @@ def run_build_examples(args): packages_dir = command_utils.get_package_dir() target_plugins, _ = command_utils.get_target_plugins( packages_dir, - plugins=args.plugins, - exclude=args.exclude, + candidates=args.plugins, + excludes=args.exclude, run_on_changed_packages=args.run_on_changed_packages, base_sha=args.base_sha) results = [] diff --git a/tools/commands/command_utils.py b/tools/commands/command_utils.py index 47f005493..75f4fd962 100755 --- a/tools/commands/command_utils.py +++ b/tools/commands/command_utils.py @@ -15,10 +15,7 @@ def set_parser_arguments(parser, nargs='*', default=[], help=f'Specifies which plugins to {command}. \ - If it is not specified and --run-on-changed-packages is also not specified, \ - then it will include every plugin under packages. \ - If both flags are specified, then --run-on-changed-packages is ignored.' - ) + If it is not specified then it will include every plugin under packages.') if exclude: parser.add_argument('--exclude', @@ -51,7 +48,7 @@ def set_parser_arguments(parser, Default is 120 seconds.') -def get_changed_plugins(packages_dir, base_sha=''): +def _get_changed_plugins(packages_dir, base_sha=''): if base_sha == '': base_sha = subprocess.run( 'git merge-base --fork-point FETCH_HEAD HEAD', @@ -87,34 +84,48 @@ def get_changed_plugins(packages_dir, base_sha=''): def get_target_plugins(packages_dir, - plugins=[], - exclude=[], + candidates=[], + excludes=[], run_on_changed_packages=False, base_sha=''): - existing_plugins = os.listdir(packages_dir) - for plugin in plugins: - if plugin not in existing_plugins: - print(f'{plugin} package does not exist, ignoring input...') - - plugin_names = [] - if len(plugins) == 0 and run_on_changed_packages: - plugin_names = get_changed_plugins(packages_dir, base_sha) - else: - for plugin_name in existing_plugins: - if not os.path.isdir(os.path.join(packages_dir, plugin_name)): - continue - if len(plugins) > 0 and plugin_name not in plugins: - continue - plugin_names.append(plugin_name) - - excluded_plugins = [] - testing_plugins = [] - for plugin_name in plugin_names: - if plugin_name in exclude: - excluded_plugins.append(plugin_name) + # If no candidates are provided, all packages under `/packages` are candidates. + existing_packages = os.listdir(packages_dir) + if not candidates: + candidates = existing_packages[:] + + # Remove non existing package names. + existing_candidates = [] + for candidate in candidates: + if candidate not in existing_packages: + print(f'{candidate} package does not exist, ignoring input...') else: - testing_plugins.append(plugin_name) - return testing_plugins, excluded_plugins + existing_candidates.append(candidate) + existing_excludes = [] + for exclude in excludes: + if exclude not in existing_packages: + print(f'{exclude} package does not exist, ignoring input...') + else: + existing_excludes.append(exclude) + + # If `run_on_changed_packages` is true, get the subset of `existing_candidates` where each package is changed. + if run_on_changed_packages: + changed_packages = _get_changed_plugins(packages_dir, base_sha) + for changed_package in changed_packages: + if changed_package not in existing_candidates: + print(f'{changed_package} package is changed but is not tested because it\'t not one of the candidates.') + existing_candidates = list(set(existing_candidates).intersection(set(changed_packages))) + + # Remove excludes that are not in `existing candidates`. + final_excludes = [] + for existing_exclude in existing_excludes: + if existing_exclude not in existing_candidates: + print(f'{existing_exclude} package does not exist in candidates, ignoring input...') + else: + final_excludes.append(existing_exclude) + + # Get the final candidates for testing by removing excludes. + final_candidates = list(set(existing_candidates).difference(set(final_excludes))) + return final_candidates, final_excludes def get_package_dir(): diff --git a/tools/commands/integration_test.py b/tools/commands/integration_test.py index f2bfd012e..45d2cacb5 100755 --- a/tools/commands/integration_test.py +++ b/tools/commands/integration_test.py @@ -569,8 +569,8 @@ def run_integration_test(args): packages_dir = command_utils.get_package_dir() testing_plugins, excluded_plugins = command_utils.get_target_plugins( packages_dir, - plugins=args.plugins, - exclude=args.exclude, + candidates=args.plugins, + excludes=args.exclude, run_on_changed_packages=args.run_on_changed_packages, base_sha=args.base_sha) diff --git a/tools/commands/requirements.txt b/tools/requirements.txt similarity index 100% rename from tools/commands/requirements.txt rename to tools/requirements.txt diff --git a/tools/tests/__init__.py b/tools/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tools/tests/test_command_utils.py b/tools/tests/test_command_utils.py new file mode 100644 index 000000000..45d008eef --- /dev/null +++ b/tools/tests/test_command_utils.py @@ -0,0 +1,90 @@ +import unittest +import os +import shutil +import subprocess + +from commands import command_utils + +class TestGetTargetPlugins(unittest.TestCase): + + def setUp(self): + self.plugins_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), 'plugins')) + self.packages_dir = os.path.join(self.plugins_dir, 'packages') + self.existing_packages = {'a', 'b', 'c', 'd', 'e'} + for package in self.existing_packages: + os.makedirs(os.path.join(self.packages_dir, package)) + + def set_up_git(self): + for package in self.existing_packages: + file_path = os.path.join(self.packages_dir, package, 'README.md') + open(file_path, 'w').close() + subprocess.run(['git', '-C', self.plugins_dir, 'init', '-q']) + subprocess.run(['git', '-C', self.plugins_dir, 'add', self.plugins_dir]) + subprocess.run(['git', '-C', self.plugins_dir, 'commit', '-q', '-m', '"Initial commit"']) + self.changed_packages = {'a', 'b', 'c'} + for package in self.changed_packages: + file_path = os.path.join(self.packages_dir, package, 'README.md') + os.remove(file_path) + subprocess.run(['git', '-C', self.plugins_dir, 'add', self.plugins_dir]) + subprocess.run(['git', '-C', self.plugins_dir, 'commit', '-q', '-m', '"Change a, b, c"']) + completed_process = subprocess.run('git rev-parse HEAD^', + shell=True, + cwd=self.plugins_dir, + universal_newlines=True, + stderr=subprocess.PIPE, + stdout=subprocess.PIPE) + self.base_sha = completed_process.stdout.strip('\n') + + def tearDown(self) -> None: + shutil.rmtree(self.plugins_dir) + + def test_passing_empty_list_returns_all_existing_plugins(self): + plugins = [] + testing_plugins, excluded_plugins = command_utils.get_target_plugins(self.packages_dir, candidates=plugins) + self.assertEqual(set(testing_plugins), self.existing_packages) + self.assertTrue(len(excluded_plugins) == 0) + + def test_ignores_not_existing_plugins(self): + plugins = ['a', 'b', 'f'] + testing_plugins, excluded_plugins = command_utils.get_target_plugins(self.packages_dir, candidates=plugins) + self.assertEqual(set(testing_plugins), {'a', 'b'}) + self.assertTrue(len(excluded_plugins) == 0) + + def test_excludes_plugins(self): + plugins = [] + excludes = ['a', 'c', 'e'] + testing_plugins, excluded_plugins = command_utils.get_target_plugins(self.packages_dir, candidates=plugins, excludes=excludes) + self.assertEqual(set(testing_plugins), {'b', 'd'}) + self.assertEqual(set(excluded_plugins), {'a', 'c', 'e'}) + + def test_ignore_excludes_that_are_not_test_candidates(self): + plugins = ['a', 'b', 'c'] + excludes = ['c', 'e'] + testing_plugins, excluded_plugins = command_utils.get_target_plugins(self.packages_dir, candidates=plugins, excludes=excludes) + self.assertEqual(set(testing_plugins), {'a', 'b'}) + self.assertEqual(set(excluded_plugins), {'c'}) + + def test_get_changed_plugins(self): + self.set_up_git() + plugins = [] + testing_plugins, excluded_plugins = command_utils.get_target_plugins(self.packages_dir, candidates=plugins, run_on_changed_packages=True, base_sha=self.base_sha) + self.assertEqual(set(testing_plugins), self.changed_packages) + self.assertTrue(len(excluded_plugins) == 0) + + def test_ignore_changed_plugins_not_in_candidate(self): + self.set_up_git() + plugins = ['a', 'b'] + testing_plugins, excluded_plugins = command_utils.get_target_plugins(self.packages_dir, candidates=plugins, run_on_changed_packages=True, base_sha=self.base_sha) + self.assertEqual(set(testing_plugins), {'a', 'b'}) + self.assertTrue(len(excluded_plugins) == 0) + + def test_exclude_changed_plugins(self): + self.set_up_git() + plugins = [] + excludes = ['a', 'c'] + testing_plugins, excluded_plugins = command_utils.get_target_plugins(self.packages_dir, candidates=plugins, excludes=excludes, run_on_changed_packages=True, base_sha=self.base_sha) + self.assertEqual(set(testing_plugins), {'b'}) + self.assertEqual(set(excluded_plugins), {'a', 'c'}) + +if __name__ == '__main__': + unittest.main()