Do not copy unused options on --insert

docs/releases/pending/insert-cli-defaults.fmf

@@ -0,0 +1,6 @@
+description: |
+  The ``--insert`` step action no longer copies unused command-line
+  defaults into the new phase. Testing Farm and similar callers of
+  ``discover --insert --how fmf --url ...`` no longer trigger false
+  deprecation warnings for the obsolete ``repository`` and
+  ``revision`` keys.

tests/unit/test_cli.py

@@ -266,3 +266,49 @@ def test_decide_colorization(
     monkeypatch.setattr(sys.stderr, 'isatty', lambda: testcase.simulate_tty)
 
     assert tmt.log.decide_colorization(no_color, force_color) == testcase.expected
+
+
+def test_discover_insert_omits_unused_cli_defaults(run_tmt: 'RunTmt', tmppath: Path) -> None:
+    """
+    ``--insert`` must not copy unused Click defaults such as ``repository``
+    and ``revision`` into the new phase.
+    """
+
+    from tmt.steps.discover import Discover
+
+    # CLI invocations are stored on the class and are not reset between
+    # tests, drop any leftovers before and after this test.
+    def _reset() -> None:
+        Discover.cli_invocations.clear()
+        Discover.cli_invocation = None
+
+    _reset()
+
+    root = tmppath / 'tree'
+    (root / '.fmf').mkdir(parents=True)
+    (root / '.fmf' / 'version').write_text('1\n')
+    (root / 'tests').mkdir()
+    (root / 'tests' / 'one.fmf').write_text('test: /bin/true\n')
+    (root / 'plans').mkdir()
+    (root / 'plans' / 'main.fmf').write_text('discover:\n    how: fmf\nexecute:\n    how: tmt\n')
+
+    try:
+        result = run_tmt(
+            '--root',
+            str(root),
+            'run',
+            '-i',
+            str(tmppath / 'run'),
+            'discover',
+            '--insert',
+            '--how',
+            'fmf',
+            '--test',
+            '/tests/one',
+        )
+    finally:
+        _reset()
+
+    assert result.exit_code == 0, result.output
+    assert "Field 'repository' is deprecated" not in result.output
+    assert "Field 'revision' is deprecated" not in result.output

tmt/steps/__init__.py

@@ -1147,17 +1147,28 @@ def _apply_cli_invocations(self, raw_data: list[_RawStepData]) -> list[_RawStepD
 
         debug1(f'Update {self.__class__.__name__.lower()} phases by CLI invocations')
 
-        def _to_raw_step_datum(options: dict[str, Any]) -> _RawStepData:
+        def _to_raw_step_datum(invocation: 'tmt.cli.CliInvocation') -> _RawStepData:
             """
             Convert CLI options to fmf-like raw step data dictionary.
 
-            This means dropping all keys that cannot come from an fmf node, like
-            keys representing CLI options.
+            Drop keys that cannot come from an fmf node, such as keys representing
+            CLI actions. Also omit options that were not really given on the
+            command line or via environment. Click fills every option with its
+            default, so a naive copy would put unused deprecated aliases such as
+            ``repository`` and ``revision`` into an ``--insert`` phase.
             """
 
             def _iter_options() -> Iterator[tuple[str, Any]]:
-                for name, value in options.items():
-                    if name in ('update', 'update_missing', 'insert', 'allowed-how'):
+                for name, value in invocation.options.items():
+                    if name in ('update', 'update_missing', 'insert', 'allowed_how'):
+                        continue
+
+                    value_source = invocation.option_sources.get(name)
+                    if value_source not in (
+                        ParameterSource.COMMANDLINE,
+                        ParameterSource.ENVIRONMENT,
+                    ):
+                        debug4(f'{name} not really given via CLI/env, omit from raw step datum')
                         continue
 
                     yield key_to_option(name), value
@@ -1196,40 +1207,26 @@ def _ensure_name(raw_datum: _RawStepData) -> _RawStepData:
         def _patch_raw_datum(
             raw_datum: _RawStepData,
             incoming_raw_datum: _RawStepData,
-            invocation: 'tmt.cli.CliInvocation',
             missing_only: bool = False,
         ) -> None:
             """
             Copy options from one phase specification onto another.
 
             Serves as a helper for "patching" a phase with options coming from
-            a command line. It must avoid copying options that were not really
-            given by user - because of how options are handled, simple
-            ``dict.update()`` would not do as ``incoming_raw_datum`` would
-            contain **all** options as long as they have a default value.
-
-            Click is therefore consulted for each key/option, whether it was
-            really specified on the command line (or by an environment
-            variable).
+            a command line. ``incoming_raw_datum`` is expected to contain only
+            options really given by the user, see :py:func:`_to_raw_step_datum`,
+            therefore this helper only needs to handle ``--update-missing``
+            semantics.
             """
 
             debug3('raw step datum', str(raw_datum))
             debug3('incoming raw step datum', str(incoming_raw_datum))
-            debug3('CLI invocation', str(invocation.options))
 
             for opt, value in incoming_raw_datum.items():
                 if opt == 'name':
                     continue
 
-                key = option_to_key(opt)
-                value_source = invocation.option_sources.get(key)
-
-                debug3(f'{opt=} {key=} {value=} {value_source=}')
-
-                # Ignore CLI input if it's been provided by option's default
-                if value_source not in (ParameterSource.COMMANDLINE, ParameterSource.ENVIRONMENT):
-                    debug4('value not really given via CLI/env, no effect')
-                    continue
+                debug3(f'{opt=} {value=}')
 
                 # Ignore CLI input if `--missing-only` has been set and datum already has the key.
                 if missing_only and opt in raw_datum:
@@ -1319,7 +1316,7 @@ def _log_raw_data(stage: str, raw_data: list[_RawStepData]) -> None:
             elif invocation.options.get('insert'):
                 debug3('inserting new phase')
 
-                raw_datum = _to_raw_step_datum(invocation.options)
+                raw_datum = _to_raw_step_datum(invocation)
                 raw_datum = _ensure_name(raw_datum)
 
                 raw_data.append(raw_datum)
@@ -1332,13 +1329,13 @@ def _log_raw_data(stage: str, raw_data: list[_RawStepData]) -> None:
                 needle = invocation.options.get('name')
 
                 if needle:
-                    incoming_raw_datum = _to_raw_step_datum(invocation.options)
+                    incoming_raw_datum = _to_raw_step_datum(invocation)
 
                     for raw_datum in raw_data:
                         if raw_datum['name'] != needle:
                             continue
 
-                        _patch_raw_datum(raw_datum, incoming_raw_datum, invocation)
+                        _patch_raw_datum(raw_datum, incoming_raw_datum)
 
                         break
 
@@ -1358,15 +1355,13 @@ def _log_raw_data(stage: str, raw_data: list[_RawStepData]) -> None:
                 needle = invocation.options.get('name')
 
                 if needle:
-                    incoming_raw_datum = _to_raw_step_datum(invocation.options)
+                    incoming_raw_datum = _to_raw_step_datum(invocation)
 
                     for raw_datum in raw_data:
                         if raw_datum['name'] != needle:
                             continue
 
-                        _patch_raw_datum(
-                            raw_datum, incoming_raw_datum, invocation, missing_only=True
-                        )
+                        _patch_raw_datum(raw_datum, incoming_raw_datum, missing_only=True)
 
                         break
 
@@ -1392,7 +1387,7 @@ def _log_raw_data(stage: str, raw_data: list[_RawStepData]) -> None:
             debug2(f'postponed invocation #{i}', str(invocation.options))
 
             pruned_raw_data: list[_RawStepData] = []
-            incoming_raw_datum = _to_raw_step_datum(invocation.options)
+            incoming_raw_datum = _to_raw_step_datum(invocation)
 
             # In the 'tmt try image' command user can specify their
             # preferred image name without specifying the provision
@@ -1438,10 +1433,10 @@ def _log_raw_data(stage: str, raw_data: list[_RawStepData]) -> None:
                     )
 
                 if invocation.options.get('update_missing'):
-                    _patch_raw_datum(raw_datum, incoming_raw_datum, invocation, missing_only=True)
+                    _patch_raw_datum(raw_datum, incoming_raw_datum, missing_only=True)
 
                 else:
-                    _patch_raw_datum(raw_datum, incoming_raw_datum, invocation)
+                    _patch_raw_datum(raw_datum, incoming_raw_datum)
 
                 pruned_raw_data.append(raw_datum)