From 00355aca85b19c72af9b14d01c24b1672897a998 Mon Sep 17 00:00:00 2001 From: Germain Haugou Date: Sat, 14 Mar 2026 22:16:04 +0100 Subject: [PATCH] fix: Correct target filtering for --target flag Two issues fixed: 1. When --target is specified, untargeted tests (from testsets without a targets section in gvtest.yaml) were still running. Now they are silently excluded at enqueue time by checking the _is_fallback flag on the target. 2. When multiple --target values are specified and they come from different gvtest.yaml files in different sub-directories, only targets from the first matching yaml were discovered. Root cause: import_testset() only fanned out to a sub-testset's targets if the parent's target matched the sub's targets. When targets come from different scopes (e.g. root has spatz, sub has pulp-open), the parent's spatz didn't match the sub's pulp-open, so the sub was skipped. Fix: sub-testsets with their own targets section always fan out independently of the parent's target. A _fanned_out set on the runner prevents duplication when multiple parent targets import the same sub-testset. Behavior: - gvtest --target X: runs only tests with target X - gvtest --target X --target Y: runs tests for both, even if X and Y come from different gvtest.yaml files - gvtest (no --target): runs everything Added 6 tests covering all target filtering scenarios. --- python/gvtest/runner.py | 18 +++ python/gvtest/tests.py | 13 +++ python/gvtest/testset_impl.py | 34 +++--- tests/test_runner.py | 200 ++++++++++++++++++++++++++++++++++ 4 files changed, 249 insertions(+), 16 deletions(-) diff --git a/python/gvtest/runner.py b/python/gvtest/runner.py index 1533d64..49fb3f7 100644 --- a/python/gvtest/runner.py +++ b/python/gvtest/runner.py @@ -136,6 +136,14 @@ def __init__( self.default_target: Target = Target('default') else: self.default_target = Target(self.target_names[0]) + # Mark the default target as a fallback — it was + # not resolved from a gvtest.yaml targets section. + self.default_target._is_fallback = True + # Track sub-testset files that have already been + # fanned out to their own targets, to prevent + # duplication when multiple parent targets import + # the same sub-testset. + self._fanned_out: set[str] = set() self.cpu_poll_interval: float = 0.1 self.report_all: bool = report_all self.progress: bool = progress @@ -406,6 +414,11 @@ def stop(self) -> None: signal.signal(signal.SIGINT, self._orig_sigint) self._orig_sigint = None + @property + def _cli_targets_specified(self) -> bool: + """True when the user explicitly passed --target.""" + return self.target_names != ['default'] + def add_testset(self, file: str) -> None: if not os.path.isabs(file): file = os.path.join(os.getcwd(), file) @@ -420,6 +433,11 @@ def add_testset(self, file: str) -> None: self.import_testset(file, target, None) ) else: + # No YAML targets at this level — load with + # default target. The testset may import + # sub-testsets that DO define targets. + # Filtering of untargeted tests happens at + # enqueue time (see TestCommon.enqueue). self.testsets.append( self.import_testset( file, self.default_target, None diff --git a/python/gvtest/tests.py b/python/gvtest/tests.py index d847c76..13bce4c 100644 --- a/python/gvtest/tests.py +++ b/python/gvtest/tests.py @@ -426,6 +426,19 @@ def enqueue(self) -> None: self.target.name if self.target is not None else self.runner.get_config() ) + + # When --target is specified, skip tests whose target + # is just the fallback (not from a gvtest.yaml + # targets section). This ensures only tests + # belonging to a real target are executed. + if (self.runner._cli_targets_specified + and self.target is not None + and getattr( + self.target, '_is_fallback', False + )): + # Don't count or show — silently excluded + return + self.runner.count_test() if self.runner.tui is not None: self.runner.tui.count_target(config) diff --git a/python/gvtest/testset_impl.py b/python/gvtest/testset_impl.py index 7b96647..ade6d55 100644 --- a/python/gvtest/testset_impl.py +++ b/python/gvtest/testset_impl.py @@ -85,29 +85,31 @@ def import_testset(self, file: str) -> None: sub_dir = os.path.dirname(filepath) if self.runner._has_own_targets(sub_dir): - # Sub-dir defines its own targets. - sub_targets = ( - self.runner._resolve_targets_for_dir(sub_dir) - ) - sub_names = [t.name for t in sub_targets] - my_name = ( - self.target.name - if hasattr(self.target, 'name') - else 'default' - ) - - if my_name == 'default' or my_name in sub_names: - # First parent to reach here, or parent's - # target matches: fan out to sub's targets + # Sub-dir defines its own targets — it owns + # its target scope independently of the parent. + # Use _fanned_out to ensure we only fan out + # once per sub-testset file (avoids duplication + # when multiple parent targets import the same + # sub-testset). + real_path = os.path.realpath(filepath) + if real_path not in self.runner._fanned_out: + self.runner._fanned_out.add(real_path) + sub_targets = ( + self.runner._resolve_targets_for_dir( + sub_dir + ) + ) for target in sub_targets: self.testsets.append( self.runner.import_testset( filepath, target, self ) ) - # else: parent's target not in sub's list → skip else: - # Inherit parent's target + # Inherit parent's target. If parent is + # "default" and --target was specified, + # the tests created inside will be filtered + # at enqueue time. self.testsets.append( self.runner.import_testset( filepath, self.target, self diff --git a/tests/test_runner.py b/tests/test_runner.py index fb3c3ed..95b4452 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -744,6 +744,206 @@ def testset_build(testset): assert r.stats.stats['passed'] == 1 + def test_cli_target_skips_untargeted_tests(self, tmp_path): + """When --target X is specified, tests without any + target definition should be skipped entirely.""" + # No gvtest.yaml → no targets defined + testset_file = tmp_path / 'testset.cfg' + testset_file.write_text(''' +from gvtest.testsuite import * + +def testset_build(testset): + testset.set_name('no_target') + test = testset.new_test('should_not_run') + test.add_command(Shell('run', 'echo bad')) +''') + r = Runner( + properties=[], flags=[], nb_threads=1, + targets=['some_target'] + ) + r.add_testset(str(testset_file)) + r.start() + r.run() + r.stop() + # No tests should have run + assert r.stats.stats['passed'] == 0 + assert r.stats.stats['failed'] == 0 + + def test_no_cli_target_runs_all(self, tmp_path): + """When no --target is specified, tests without + targets should run normally.""" + testset_file = tmp_path / 'testset.cfg' + testset_file.write_text(''' +from gvtest.testsuite import * + +def testset_build(testset): + testset.set_name('default_run') + test = testset.new_test('should_run') + test.add_command(Shell('run', 'echo ok')) +''') + r = Runner(properties=[], flags=[], nb_threads=1) + r.add_testset(str(testset_file)) + r.start() + r.run() + r.stop() + assert r.stats.stats['passed'] == 1 + + def test_cli_target_runs_only_matching(self, tmp_path): + """When --target X is specified, only tests with + target X should run, not untargeted tests.""" + config = tmp_path / 'gvtest.yaml' + config.write_text( + 'targets:\n target_a: {}\n target_b: {}\n' + ) + + testset_file = tmp_path / 'testset.cfg' + testset_file.write_text(''' +from gvtest.testsuite import * + +def testset_build(testset): + testset.set_name('multi') + test = testset.new_test('targeted_test') + test.add_command(Shell('run', 'echo ok')) +''') + # Request only target_a + r = Runner( + properties=[], flags=[], nb_threads=1, + targets=['target_a'] + ) + r.add_testset(str(testset_file)) + r.start() + r.run() + r.stop() + # Should run once (target_a only, not target_b) + assert r.stats.stats['passed'] == 1 + + def test_cli_target_with_sub_testset_targets( + self, tmp_path + ): + """When --target X is specified, root has no targets + but a sub-testset defines X, the sub's tests should + run and root's direct tests should be skipped.""" + # Sub-testset with targets + sub_dir = tmp_path / 'sub' + sub_dir.mkdir() + sub_config = sub_dir / 'gvtest.yaml' + sub_config.write_text( + 'targets:\n spatz_v2: {}\n rv64: {}\n' + ) + sub_testset = sub_dir / 'testset.cfg' + sub_testset.write_text(''' +from gvtest.testsuite import * + +def testset_build(testset): + testset.set_name('sub') + test = testset.new_test('targeted_test') + test.add_command(Shell('run', 'echo targeted')) +''') + # Root testset imports sub, also has its own test + root_testset = tmp_path / 'testset.cfg' + root_testset.write_text(''' +from gvtest.testsuite import * + +def testset_build(testset): + testset.set_name('root') + test = testset.new_test('untargeted_test') + test.add_command(Shell('run', 'echo untargeted')) + testset.import_testset('sub/testset.cfg') +''') + r = Runner( + properties=[], flags=[], nb_threads=1, + targets=['spatz_v2'] + ) + r.add_testset(str(root_testset)) + r.start() + r.run() + r.stop() + # Only the sub's test for spatz_v2 should run, + # not the root's untargeted test + assert r.stats.stats['passed'] == 1 + + def test_multi_target_across_yaml_files( + self, tmp_path + ): + """When --target A --target B and A/B come from + different gvtest.yaml files, both should run.""" + # Sub1 has target_a + sub1 = tmp_path / 'sub1' + sub1.mkdir() + (sub1 / 'gvtest.yaml').write_text( + 'targets:\n target_a: {}\n' + ) + (sub1 / 'testset.cfg').write_text(''' +from gvtest.testsuite import * + +def testset_build(testset): + testset.set_name('sub1') + test = testset.new_test('test_a') + test.add_command(Shell('run', 'echo a')) +''') + # Sub2 has target_b + sub2 = tmp_path / 'sub2' + sub2.mkdir() + (sub2 / 'gvtest.yaml').write_text( + 'targets:\n target_b: {}\n' + ) + (sub2 / 'testset.cfg').write_text(''' +from gvtest.testsuite import * + +def testset_build(testset): + testset.set_name('sub2') + test = testset.new_test('test_b') + test.add_command(Shell('run', 'echo b')) +''') + # Root imports both + root = tmp_path / 'testset.cfg' + root.write_text(''' +from gvtest.testsuite import * + +def testset_build(testset): + testset.set_name('root') + testset.import_testset('sub1/testset.cfg') + testset.import_testset('sub2/testset.cfg') +''') + r = Runner( + properties=[], flags=[], nb_threads=1, + targets=['target_a', 'target_b'] + ) + r.add_testset(str(root)) + r.start() + r.run() + r.stop() + # Both sub-testsets should run (1 test each) + assert r.stats.stats['passed'] == 2 + + def test_no_cli_target_runs_all_yaml_targets( + self, tmp_path + ): + """When no --target is specified and YAML defines + targets, all targets should run.""" + config = tmp_path / 'gvtest.yaml' + config.write_text( + 'targets:\n target_a: {}\n target_b: {}\n' + ) + + testset_file = tmp_path / 'testset.cfg' + testset_file.write_text(''' +from gvtest.testsuite import * + +def testset_build(testset): + testset.set_name('all_targets') + test = testset.new_test('run_both') + test.add_command(Shell('run', 'echo ok')) +''') + r = Runner(properties=[], flags=[], nb_threads=1) + r.add_testset(str(testset_file)) + r.start() + r.run() + r.stop() + # Should run twice (once per target) + assert r.stats.stats['passed'] == 2 + + # --------------------------------------------------------------------------- # Config integration (gvtest.yaml + testset loading) # ---------------------------------------------------------------------------