Add `link` command and linking to jira on new issues

[tkoscieln]

Pull Request Checklist

• [x] implement the feature
• [x] write the documentation
• [x] extend the test coverage
• [ ] update the specification
• [ ] adjust plugin docstring
• [ ] modify the json schema
• [ ] mention the version
• [ ] include a release note

[LecrisUT]

Could use an example of how this works. Probably this needs to be abstracted a bit for more arbitrary issue providers. Are there no plans yet to strip out some of the tmt plugins in order to minimize the dependencies and tests?

[tkoscieln]

Could use an example of how this works. Probably this needs to be abstracted a bit for more arbitrary issue providers. Are there no plans yet to strip out some of the tmt plugins in order to minimize the dependencies and tests?

This is currently WIP, hence why it is in Draft. Will provide more information once the initial refactor suggested by @psss in a discussion will be implemented.

[tkoscieln]

Added a section to documentation which should explain what this change aims to introduce. Related Jira issue TT-262.

[LecrisUT]

Thanks for the clarification I kinda understand the setup, and I am more confused about it at the same time. The main idea is to add in the Issue links a link to the tmt test/plan/story that would fix a specific issue. The confusing part is the usage of the service field. This seems to imply that the tmt metadata is served somehow probably as teemtee/web? That part should be advertised a bit more if that is the only linkage that would be supported. What about general audience?

Another question I have is what happens with the links when a test/structure gets refactored. Could the tests be traced back through the git tree to see how it evolves and re-links?

[psss]

This is an important feature blocking proper Jira integration and traceability. Marking as a must have. Let's try hard to finish this in 1.36..

[psss]

@tkoscieln, could you please address the comments and add a short release note? Thanks!

[psss]

@tkoscieln, any update here?

[tkoscieln]

@tkoscieln, any update here?

Working currently on the link being written into the metadata on the disk. That should be the last "major" feature.

[psss]

Seems there is a bunch of tests failing, some of them with a very basic functionality:

tmt --help
list index out of range

@tkoscieln, could you have a look please?

[tkoscieln]

tmt --help
list index out of range

This error was caused by missing docstring in link command implementation. Fixed in most recent commit.

[tkoscieln]

I have added the ModuleImporter, moved the linking to a separate module and while the unit tests are passing, I am unable to resolve some of the failures in pre-commit check. Could someone have a look? (@happz ?)

[happz]

I have added the ModuleImporter, moved the linking to a separate module and while the unit tests are passing, I am unable to resolve some of the failures in pre-commit check. Could someone have a look? (@happz ?)

Sure, although there's still a conflict with tmt/cli.py, and pre-commit does not seem to start :/

[tkoscieln]

I have added the ModuleImporter, moved the linking to a separate module and while the unit tests are passing, I am unable to resolve some of the failures in pre-commit check. Could someone have a look? (@happz ?)

Sure, although there's still a conflict with tmt/cli.py, and pre-commit does not seem to start :/

I have rebased this branch onto main, so the conflict should be fixed

[psss]

Tried to fix traceback for weird debug level in 0c630b78 but ruff is not happy. @happz, any suggestion how to better fix the uncatched exception without moving the import commands?

[martinhoyer]

Tried to fix traceback for weird debug level in 0c630b7 but ruff is not happy. @happz, any suggestion how to better fix the uncatched exception without moving the import commands?

huh, if I understand it correctly, this could only happen when TMT_DEBUG is set to weird? :confused:

[LecrisUT]

Sounds familiar I've encountered this one as well, I think with https://github.com/teemtee/tmt/pull/2946? That one has a different proposed solution though, see tmt/log.py there.

[happz]

Tried to fix traceback for weird debug level in 0c630b7 but ruff is not happy. @happz, any suggestion how to better fix the uncatched exception without moving the import commands?

How can I trigger that traceback?

[happz]

Tried to fix traceback for weird debug level in 0c630b7 but ruff is not happy. @happz, any suggestion how to better fix the uncatched exception without moving the import commands?

Doesn't traceback for me, I'm afraid, only if I force TMT_DEBUG=weird:

(dev) [pts-5:0]: happz@multivac [(HEAD detached at f52d782a)] ~/git/tmt $ tmt
Found 200 tests: /tests/provision/virtual/dependencies, /tests/artemis/noop, /tests/backward-compatibility, /tests/clean/basic, /tests/clean/chain, /tests/clean/guests, /tests/clean/runs, /tests/core/adjust, /tests/core/context, /tests/core/debug, /tests/core/docs, /tests/core/dry and 188 more.
Found 25 plans: /plans/features/advanced, /plans/features/basic, /plans/features/core, /plans/features/extended-unit-tests, /plans/features/steps/discover, /plans/features/steps/execute, /plans/features/steps/prepare, /plans/features/steps/the-rest, /plans/install/minimal, /plans/install/pip/full, /plans/install/pip/mini, /plans/integration and 13 more.
Found 193 stories: /spec/context/dimension, /spec/context/initiator, /spec/context/trigger, /spec/core/adjust, /spec/core/contact, /spec/core/description, /spec/core/enabled, /spec/core/id, /spec/core/link, /spec/core/order, /spec/core/summary, /spec/core/tag and 181 more.
(dev) [pts-5:0]: happz@multivac [(HEAD detached at f52d782a)] ~/git/tmt $ TMT_DEBUG=weird tmt
Traceback (most recent call last):
  File "/home/happz/git/tmt/tmt/log.py", line 120, in _debug_level_from_global_envvar
    return int(raw_value)
           ^^^^^^^^^^^^^^
ValueError: invalid literal for int() with base 10: 'weird'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/happz/.local/share/hatch/env/virtual/tmt/BuhwJjvK/dev/bin/tmt", line 5, in <module>
    from tmt.__main__ import run_cli
  File "/home/happz/git/tmt/tmt/__init__.py", line 20, in <module>
    from tmt.base import Clean, Plan, Run, Status, Story, Test, Tree
  File "/home/happz/git/tmt/tmt/base.py", line 57, in <module>
    import tmt.utils.jira
  File "/home/happz/git/tmt/tmt/utils/jira.py", line 21, in <module>
    tmt.log.Logger.get_bootstrap_logger())
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/happz/git/tmt/tmt/log.py", line 871, in get_bootstrap_logger
    cls._bootstrap_logger = Logger.create(actual_logger=actual_logger)
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/happz/git/tmt/tmt/log.py", line 686, in create
    return Logger(
           ^^^^^^^
  File "/home/happz/git/tmt/tmt/log.py", line 629, in apply_verbosity_options
    debug_level_from_global_envvar = _debug_level_from_global_envvar()
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/happz/git/tmt/tmt/log.py", line 123, in _debug_level_from_global_envvar
    raise tmt.utils.GeneralError(f"Invalid debug level '{raw_value}', use an integer.")
tmt.utils.GeneralError: Invalid debug level 'weird', use an integer.
(dev) [pts-5:0]: happz@multivac [(HEAD detached at f52d782a)] ~/git/tmt $ 

Could you have this envvar set somehow? It's set in one of the tests, I'd be surprised if it leaked somehow, but other than this, I see no way how the traceback could report the debug level to be, literally, weird. The attempt to address it does not look like it would actually remove the cause, just mask it by postponing the imports.

[psss]

How can I trigger that traceback?

TMT_DEBUG=weird tmt plan show

With main or with the fix in 0c630b78 it just prints the error message as expected:

Error: failed while importing tmt package: Invalid debug level 'weird', use an integer.

But without moving the import statements it tracebacks. I wonder if there is a better way to fix this. Seems to be related to the order in which modules are imported.

[LecrisUT]

Try out b09d66bbc9913e17f4dc08e97b80cb18c46817a1 to make _bootstrap_logger safe for arbitrary import orders. It moves the TMT_DEBUG check to click and in bootstrap it has a fallback to 0 if it was invalid

[happz]

How can I trigger that traceback?

TMT_DEBUG=weird tmt plan show

With main or with the fix in 0c630b7 it just prints the error message as expected:

Error: failed while importing tmt package: Invalid debug level 'weird', use an integer.

Ah, ok, so you are running the test... Let me try.

But without moving the import statements it tracebacks. I wonder if there is a better way to fix this. Seems to be related to the order in which modules are imported.

Yes, it was already encountered elsewhere, as mentioned by @LecrisUT. There is definitely a better way than repeating imports, it'd be nice to address it in its own patch though...

[happz]

Try out b09d66b to make _bootstrap_logger safe for arbitrary import orders. It moves the TMT_DEBUG check to click and in bootstrap it has a fallback to 0 if it was invalid

Yeah, and I recall having one or two comments against that approach :/

I'm reading the code once again, possibly fully realizing what went wrong, and it seems to me the logger is not really needed at all :O Let me draft a quick patch, it's needed when importing the package, but not in the import time of tmt itself.

[happz]

@psss can you try it out with reverted last commit & the following applied?

diff --git a/tmt/plugins/__init__.py b/tmt/plugins/__init__.py
index 7c987e17..5d8277e0 100644
--- a/tmt/plugins/__init__.py
+++ b/tmt/plugins/__init__.py
@@ -389,22 +389,20 @@ class ModuleImporter(Generic[ModuleT]):
             self,
             module: str,
             exc_class: type[Exception],
-            exc_message: str,
-            logger: Logger) -> None:
+            exc_message: str) -> None:
         self._module_name = module
         self._exc_class = exc_class
         self._exc_message = exc_message
-        self._logger = logger
 
         self._module: Optional[ModuleT] = None
 
-    def __call__(self) -> ModuleT:
+    def __call__(self, logger: Logger) -> ModuleT:
         if self._module is None:
             self._module = _import_or_raise(
                 module=self._module_name,
                 exc_class=self._exc_class,
                 exc_message=self._exc_message,
-                logger=self._logger)
+                logger=logger)
 
         assert self._module  # narrow type
         return self._module
diff --git a/tmt/steps/report/junit.py b/tmt/steps/report/junit.py
index 234f66f1..22ca429a 100644
--- a/tmt/steps/report/junit.py
+++ b/tmt/steps/report/junit.py
@@ -46,8 +46,7 @@ DEFAULT_TEMPLATE_DIR = Path('steps/report/junit/templates/')
 import_lxml: ModuleImporter['lxml'] = ModuleImporter(  # type: ignore[valid-type]
     'lxml',
     tmt.utils.ReportError,
-    "Missing 'lxml', fixable by 'pip install tmt[report-junit]'.",
-    tmt.log.Logger.get_bootstrap_logger())
+    "Missing 'lxml', fixable by 'pip install tmt[report-junit]'.")
 
 
 @overload
diff --git a/tmt/utils/jira.py b/tmt/utils/jira.py
index 398ba04f..79a04f23 100644
--- a/tmt/utils/jira.py
+++ b/tmt/utils/jira.py
@@ -17,8 +17,7 @@ if TYPE_CHECKING:
 import_jira: ModuleImporter['jira'] = ModuleImporter(  # type: ignore[valid-type]
     'jira',
     tmt.utils.ReportError,
-    "Install 'tmt+link-jira' to use the Jira linking.",
-    tmt.log.Logger.get_bootstrap_logger())
+    "Install 'tmt+link-jira' to use the Jira linking.")
 
 
 def jira_link(
@@ -27,7 +26,7 @@ def jira_link(
         logger: tmt.log.Logger,
         separate: bool = False) -> None:
     """ Link the object to Jira issue and create the URL to tmt web service """
-    jira_module = import_jira()
+    jira_module = import_jira(logger)
 
     def create_url_params(tmt_object: tmt.base.Core) -> dict[str, Any]:
         tmt_type = tmt_object.__class__.__name__.lower()

[LecrisUT]

@happz I had looked at the ModuleImporter and it kinda bothers me, mainly because it doesn't play well with IDE navigation and the typehinting can get quite wonky. I have uploaded a minimum structure for a possible alternative design: https://github.com/LecrisUT/optional_imports

The idea is to have tmt.optional_moduels.{lxml,jira} that are dynamically imported inside the functions, e.g.:

def jira_link(
        nodes: Sequence[Union['tmt.base.Test', 'tmt.base.Plan', 'tmt.base.Story']],
        links: 'tmt.base.Links',
        logger: tmt.log.Logger,
        separate: bool = False) -> None:
    """ Link the object to Jira issue and create the URL to tmt web service """
    ## Current approach 
    # jira_module = import_jira()
    ## Proposal
    from tmt.optional_modules.jira import jira as jira_module

Inside tmt/optional_modules/jira.py you would have the current definition:

jira: ModuleImporter['jira'] = ModuleImporter(  # type: ignore[valid-type]
    'jira',
    tmt.utils.ReportError,
    "Install 'tmt+link-jira' to use the Jira linking.",
    tmt.log.Logger.get_bootstrap_logger())

but then you can add a stub file tmt/optional_modules/jira.pyi where you mask the jira module to what it would actually be:

import jira as real_jira

jira = real_jira

I think we can further simplify the interface by overriding __dir__ and __all__ so that we don't need to reference jira.jira like that (edit: played around with it and indeed it's possible). This is compatible with PyCharm and mypy in that the auto-completion and type-hinting are taken directly from the available jira module, and this resolves the bootstrap logger issue because the import would only run within the functions and not at import time. Moving the logger would be more problematic in this case though, I haven't figured a way to neatly support that other than having a global ModuleImporter and changing its logger before the imports. Maybe if there was a method get_current_logger that would also work.

[happz]

@happz I had looked at the ModuleImporter and it kinda bothers me, mainly because it doesn't play well with IDE navigation...

It plays nicely with my VSCode instance! :) This is surprising, because mypy and pyright running under pre-commit supervision do not report types as well as VSCode does :/

... and the typehinting can get quite wonky.

I agree, partially because ModuleType is not supported by mypy and pyright to be used like this, yet again, VSCode does not mind and decodes it nicely...

I have uploaded a minimum structure for a possible alternative design: https://github.com/LecrisUT/optional_imports

Let's move it to a separate issue.

[happz]

@psss the fix for module import: https://github.com/teemtee/tmt/pull/3240

[psss]

@tkoscieln, the remaining failure in core plans are the newly added tests:

FAILED test_utils.py::TestJiraLink::test_create_link_relation - AssertionError: assert

http://localhost:8000/?format=html&test-url=https%3A%2F%2Fgithub.com%2Fteemtee%2Ftmt.git&test-name=%2Ftests%2Ftmp%2Ftest&test-ref=link-issues-to-jira
http://localhost:8000/?format=html&test-url=https%3A%2F%2Fgithub.com%2Fteemtee%2Ftmt&test-name=%2Ftests%2Funit%2Ftmp%2Ftest&test-ref=gluetool%2F6e5b8b367df61957a386a1e220693539e2a01560'

There are differences in the test-url and test-ref (expected) but also test-name differs, e.g.

test-name=/tests/tmp/test
test-name=/tests/unit/tmp/test

[psss]

@psss the fix for module import: #3240

Thanks a lot! Merged and rebased on it.

[tkoscieln]

@tkoscieln, the remaining failure in core plans are the newly added tests:

FAILED test_utils.py::TestJiraLink::test_create_link_relation - AssertionError: assert

http://localhost:8000/?format=html&test-url=https%3A%2F%2Fgithub.com%2Fteemtee%2Ftmt.git&test-name=%2Ftests%2Ftmp%2Ftest&test-ref=link-issues-to-jira
http://localhost:8000/?format=html&test-url=https%3A%2F%2Fgithub.com%2Fteemtee%2Ftmt&test-name=%2Ftests%2Funit%2Ftmp%2Ftest&test-ref=gluetool%2F6e5b8b367df61957a386a1e220693539e2a01560'

There are differences in the test-url and test-ref (expected) but also test-name differs, e.g.

test-name=/tests/tmp/test
test-name=/tests/unit/tmp/test

Hmm, I see that comparing the full urls in the context of the unit tests being run in pipelines is impossible if the refs are going to be changing e.g.. Do we want to change the assertions to something more simple, split them to individual fmf id keys? @psss Example: assert (test-url=https%3A%2F%2Fgithub.com%2Fteemtee) in result['url']

[psss]

Do we want to change the assertions to something more simple, split them to individual fmf id keys?

Yeap, something like this sounds reasonable.