Rewrite under test with again and failed-only

Skip Discover.load() for runs executed with again to reload data

docs/guide.rst

@@ -913,10 +913,10 @@ Rerunning failed tests of previously executed runs
 ------------------------------------------------------------------
 
 Executing failed tests again after fixing them is now possible
-with `tmt run` argument `--rerun`.
+with `tmt run --again test --failed-only`.
 
 This is only possible when you have the run directory available
-and `--id` argument provided (or use ) as it needs the data from execute step
+and `--id` argument provided (or use `--last`) as it needs the data from execute step
 to select only failed test cases. After new execute step, tmt will
 again merge the results from the previous run with the new ones
 to keep all the data for full report.
@@ -926,7 +926,7 @@ to keep all the data for full report.
     $ tmt run --all
     # Some tests fail, some pass
 
-    $ tmt run --last --rerun
+    $ tmt run --last --again tests --failed-only
     # Failed tests are rerun
 
 

docs/releases.rst

@@ -4,6 +4,13 @@
     Releases
 ======================
 
+tmt-1.34
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Added option ``--failed-only`` to the test attributes,
+enabling reruning failed tests from previous runs.
+
+
 tmt-1.33
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

tests/run/again/failed-only/main.fmf

@@ -0,0 +1,2 @@
+summary: Check rerun is working correctly
+tier: 3

tests/rerun/test.sh → tests/run/again/failed-only/test.sh

@@ -10,25 +10,21 @@ rlJournalStart
         rlRun "tmt init"
     rlPhaseEnd
 
-    rlPhaseStartTest "Initial run with empty run directory and rerun argument"
-        rlRun -s "tmt run --all --rerun --id $run" 2 "Rerun tests with empty directory"
-        rlAssertGrep "Run id has to be specified" $rlRun_LOG
-    rlPhaseEnd
-
-    rlPhaseStartTest "Test basic rerun scenario is working"
+    rlPhaseStartTest "Test basic run failed tests again scenario is working"
         rlRun -s "tmt run --all --scratch --id $run" 1 "Run tests, some fail"
         rlAssertGrep "total: 2 tests passed and 2 tests failed" $rlRun_LOG
 
         rlRun "sed -i 's/false/true/g' *" 0 "Fix the test"
 
-        rlRun -s "tmt run --all --rerun --id $run" 0 "Rerun failed tests"
+        rlRun -s "tmt run --all --again --id $run tests --failed-only" 0 "Rerun failed tests"
         rlAssertGrep "1 test selected" $rlRun_LOG
         rlAssertGrep "total: 4 tests passed" $rlRun_LOG
     rlPhaseEnd
 
-    rlPhaseStartTest "Test another rerun does not execute anything"
-        rlRun -s "tmt run --all --rerun --id $run" 0 "Rerun failed tests again"
+    rlPhaseStartTest "Test another run with again does not execute anything"
+        rlRun -s "tmt run --all --again --id $run tests --failed-only" 0 "Rerun failed tests again"
         rlAssertGrep "0 tests selected" $rlRun_LOG
+        rlAssertNotGrep "1 tests selected" $rlRun_LOG
         rlAssertGrep "total: 4 tests passed" $rlRun_LOG
     rlPhaseEnd
 

tmt/base.py

@@ -1669,21 +1669,6 @@ def __init__(
 
         self._plan_environment = Environment()
 
-        # Set directory for last run execute data in case of a rerun
-        if self.is_rerun:
-            assert self.workdir is not None  # narrow type
-            self.last_run_execute: Path = self.workdir / 'last_run_execute'
-
-        # Store 'environment' and 'environment-file' keys content
-        self._environment = tmt.utils.environment_from_spec(
-            raw_fmf_environment_files=node.get("environment-file") or [],
-            raw_fmf_environment=node.get('environment', {}),
-            raw_cli_environment_files=self.opt('environment-file') or [],
-            raw_cli_environment=self.opt('environment'),
-            file_root=Path(node.root) if node.root else None,
-            key_address=node.name,
-            logger=self._logger)
-
         # Expand all environment and context variables in the node
         with self.environment.as_environ():
             expand_node_data(node.data, self._fmf_context)
@@ -2248,23 +2233,6 @@ def _lint_step(step: str) -> LinterReturn:
     def wake(self) -> None:
         """ Wake up all steps """
 
-        # Additional debug info like plan environment
-        self.debug('info', color='cyan', shift=0, level=3)
-        # TODO: something better than str()?
-        self.debug('environment', format_value(self.environment), 'magenta', level=3)
-        self.debug('context', format_value(self._fmf_context), 'magenta', level=3)
-
-        # Save last run execute step if called with rerun
-        if self.is_rerun:
-            assert self.workdir is not None  # narrow type
-            if not (self.workdir / 'execute').exists():
-                raise tmt.utils.GeneralError(
-                    "Run id has to be specified and "
-                    "execute directory has to exist in order to use --rerun.")
-            self.debug(f"Saving last run execute into {self.last_run_execute}.")
-            shutil.copytree(self.workdir / 'execute', self.last_run_execute, dirs_exist_ok=True)
-
-        # Wake up all steps
         self.debug('wake', color='cyan', shift=0, level=2)
         for step in self.steps(enabled_only=False):
             self.debug(str(step), color='blue', level=2)
@@ -2335,10 +2303,9 @@ def go(self) -> None:
         try:
             for step in self.steps(skip=['finish']):
                 step.go()
-                # Finish plan if no tests found (except dry mode and rerun)
+                # Finish plan if no tests found (except dry mode)
                 if (isinstance(step, tmt.steps.discover.Discover) and not step.tests()
-                        and not self.is_dry_run and not step.extract_tests_later
-                        and not self.is_rerun):
+                        and not self.is_dry_run and not step.extract_tests_later):
                     step.info(
                         'warning', 'No tests found, finishing plan.',
                         color='yellow', shift=1)

tmt/cli.py

@@ -383,9 +383,6 @@ def main(
 @option(
     '--scratch', is_flag=True,
     help='Remove the run workdir before executing to start from scratch.')
-@option(
-    '--rerun', is_flag=True,
-    help='Rerun failed tests and update existing results.')
 @option(
     '--follow', is_flag=True,
     help='Output the logfile as it grows.')
@@ -485,6 +482,9 @@ def run_plans(context: Context, **kwargs: Any) -> None:
     help="""
          Filter by linked objects (regular expressions are supported for both relation and target).
          """)
+@option(
+    '--failed-only', is_flag=True, default=False,
+    help="Filter failed tests from a previous run to run again")
 @verbosity_options
 def run_tests(context: Context, **kwargs: Any) -> None:
     """

tmt/steps/__init__.py

@@ -628,6 +628,10 @@ def summary(self) -> None:
 
     def load(self) -> None:
         """ Load status and step data from the workdir """
+        if self.should_run_again and isinstance(self, tmt.steps.discover.Discover):
+            self.debug('Run discover again when reexecuting to capture changes in plans')
+            return
+
         try:
             raw_step_data: dict[Any, Any] = tmt.utils.yaml_to_dict(self.read(Path('step.yaml')))
 
@@ -664,7 +668,7 @@ def wake(self) -> None:
         """ Wake up the step (process workdir and command line) """
         # Cleanup possible old workdir if called with --force, but not
         # if running the step --again which should reuse saved step data
-        if (self.is_forced_run or self.is_rerun) and not self.should_run_again:
+        if self.is_forced_run and not self.should_run_again:
             self._workdir_cleanup()
 
         # Load stored data

tmt/steps/discover/__init__.py

@@ -17,7 +17,6 @@
 import tmt.utils
 from tmt.options import option
 from tmt.plugins import PluginRegistry
-from tmt.result import Result
 from tmt.steps import Action
 from tmt.utils import GeneralError, Path, field, key_to_option
 
@@ -157,37 +156,6 @@ def post_dist_git(self, created_content: list[Path]) -> None:
         """ Discover tests after dist-git applied patches """
         pass
 
-    def filter_for_rerun(self) -> None:
-        """ Filter out passed tests from previous run data """
-        assert isinstance(self.step.parent, tmt.base.Plan)  # narrow type
-        old_results: Path = self.step.parent.last_run_execute / 'results.yaml'
-        results = [
-            Result.from_serialized(data) for data in
-            tmt.utils.yaml_to_list(self.read(old_results))]
-        results_failed: list[str] = []
-        results_passed: list[Result] = []
-        for result in results:
-            if (
-                    result.result is not tmt.result.ResultOutcome.PASS and
-                    result.result is not tmt.result.ResultOutcome.INFO):
-                results_failed.append(result.name)
-            else:
-                results_passed.append(result)
-
-        # Overwrite previous run results to only include passed cases
-        self.debug(
-            f"Overwriting {old_results} to only include passed results: "
-            f"{', '.join([result.name for result in results_passed])}")
-        self.write(
-            old_results,
-            tmt.utils.dict_to_yaml([result.to_serialized() for result in results_passed]))
-
-        tests_to_execute: list[tmt.base.Test] = []
-        for test in self._tests:
-            if test.name in results_failed:
-                tests_to_execute.append(test)
-        self._tests: list[tmt.base.Test] = tests_to_execute
-
 
 class Discover(tmt.steps.Step):
     """ Gather information about test cases to be executed. """
@@ -206,6 +174,7 @@ def __init__(
 
         # Collection of discovered tests
         self._tests: dict[str, list[tmt.Test]] = {}
+        self._failed_tests: dict[str, list[tmt.Test]] = {}
 
         # Test will be (re)discovered in other phases/steps
         self.extract_tests_later: bool = False
@@ -268,44 +237,6 @@ def save(self) -> None:
 
         self.write(Path('tests.yaml'), tmt.utils.dict_to_yaml(raw_test_data))
 
-    def _filter_for_rerun(self) -> None:
-        """ Filter out passed tests from previous run data """
-        assert isinstance(self.parent, tmt.base.Plan)  # narrow type
-        old_results: Path = self.parent.last_run_execute / 'results.yaml'
-        results = [
-            Result.from_serialized(data) for data in
-            tmt.utils.yaml_to_list(self.read(old_results))]
-        results_failed: list[Result] = []
-        results_passed: list[Result] = []
-        for result in results:
-            if (
-                    result.result is not tmt.result.ResultOutcome.PASS and
-                    result.result is not tmt.result.ResultOutcome.INFO):
-                results_failed.append(result)
-            else:
-                results_passed.append(result)
-
-        # Save positive results to specific results.yaml
-        old_results_positive: Path = (
-            self.parent.last_run_execute / 'positive_results.yaml')
-        self.debug(
-            f"Save positive results from last run to {old_results_positive}, these are: "
-            f"{', '.join([result.name for result in results_passed])}")
-        self.write(
-            old_results_positive,
-            tmt.utils.dict_to_yaml([result.to_serialized() for result in results_passed]))
-
-        # Filter out failed tests based on test name and serial number
-        filtered_tests: dict[str, list[tmt.base.Test]] = {}
-        for phase in self._tests:
-            current_phase_filtered: list[tmt.base.Test] = []
-            for test in self._tests[phase]:
-                for result in results_failed:
-                    if test.name == result.name and test.serial_number == result.serial_number:
-                        current_phase_filtered.append(test)
-            filtered_tests[phase] = current_phase_filtered
-        self._tests = filtered_tests
-
     def _discover_from_execute(self) -> None:
         """ Check the execute step for possible shell script tests """
 
@@ -428,10 +359,6 @@ def go(self, force: bool = False) -> None:
         for test in self.tests():
             test.serial_number = self.plan.draw_test_serial_number(test)
 
-        # Filter selected tests if this is a rerun
-        if self.is_rerun:
-            self._filter_for_rerun()
-
         # Show fmf identifiers for tests discovered in plan
         # TODO: This part should go into the 'fmf.py' module
         if self.opt('fmf_id'):
@@ -454,6 +381,29 @@ def go(self, force: bool = False) -> None:
                 click.echo(''.join(export_fmf_ids), nl=False)
             return
 
+        if self.should_run_again:
+            if tmt.base.Test._opt('failed_only'):
+                failed_results: list[tmt.base.Result] = []
+                assert self.parent is not None  # narrow type
+                assert isinstance(self.parent, tmt.base.Plan)  # narrow type
+
+                # Get failed results from previous run execute
+                for result in self.parent.execute._results:
+                    if (
+                            result.result is not tmt.result.ResultOutcome.PASS and
+                            result.result is not tmt.result.ResultOutcome.INFO):
+                        failed_results.append(result)
+
+                # Filter existing tests into another variable which is then used by tests() method
+                for phase in self._tests:
+                    self._failed_tests[phase]: list[tmt.base.Test] = []
+                    for test in self._tests[phase]:
+                        for result in failed_results:
+                            if (
+                                    test.name == result.name and
+                                    test.serial_number == result.serial_number):
+                                self._failed_tests[phase].append(test)
+
         # Give a summary, update status and save
         self.summary()
         self.status('done')
@@ -465,13 +415,15 @@ def tests(
             phase_name: Optional[str] = None,
             enabled: Optional[bool] = None) -> list['tmt.Test']:
         def _iter_all_tests() -> Iterator['tmt.Test']:
-            for phase_tests in self._tests.values():
+            tests = self._failed_tests if self._failed_tests else self._tests
+            for phase_tests in tests.values():
                 yield from phase_tests
 
         def _iter_phase_tests() -> Iterator['tmt.Test']:
             assert phase_name is not None
+            tests = self._failed_tests if self._failed_tests else self._tests
 
-            yield from self._tests[phase_name]
+            yield from tests[phase_name]
 
         iterator = _iter_all_tests if phase_name is None else _iter_phase_tests
 

tmt/steps/discover/shell.py

@@ -443,11 +443,7 @@ def go(self) -> None:
         self._tests = tmt.Tree(
             logger=self._logger,
             tree=tests).tests(
-            conditions=["manual is False"])
-
-        # Filter selected tests if this is a rerun
-        if self.is_rerun:
-            self.filter_for_rerun()
+                conditions=["manual is False"])
 
         # Propagate `where` key and TMT_SOURCE_DIR
         for test in self._tests: