Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve fail message when library doesn't exist #2839

Merged
merged 1 commit into from May 6, 2024
Merged

Conversation

lukaszachy
Copy link
Collaborator

@lukaszachy lukaszachy commented Apr 9, 2024

When /foo doesn't exist in the fmf root tmt should provide better error than saying AttributeError: 'NoneType' object has no attribute 'data'

Pull Request Checklist

  • implement the feature
  • extend the test coverage

@lukaszachy
Copy link
Collaborator Author

Previous TB was

/var/tmp/tmt/run-287

/default/plan
    discover
        how: fmf
        directory: /tmp/r_171158_diE

plan failed

    Traceback (most recent call last):
      File "/usr/lib/python3.12/site-packages/tmt/__main__.py", line 18, in run_cli
        tmt.cli.main()
      File "/usr/lib/python3.12/site-packages/click/core.py", line 1130, in __call__
        return self.main(*args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/click/core.py", line 1055, in main
        rv = self.invoke(ctx)
             ^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/click/core.py", line 1657, in invoke
        return _process_result(sub_ctx.command.invoke(sub_ctx))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/click/core.py", line 1689, in invoke
        return _process_result(rv)
               ^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/click/core.py", line 1626, in _process_result
        value = ctx.invoke(self._result_callback, value, **ctx.params)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/click/core.py", line 760, in invoke
        return __callback(*args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/click/decorators.py", line 26, in new_func
        return f(get_current_context(), *args, **kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.12/site-packages/tmt/cli.py", line 508, in finito
        click_context.obj.run.go()
      File "/usr/lib/python3.12/site-packages/tmt/base.py", line 3560, in go
        raise tmt.utils.GeneralError(
    tmt.utils.GeneralError: plan failed

The exception was caused by 1 earlier exceptions

Cause number 1:

    'NoneType' object has no attribute 'data'

        Traceback (most recent call last):
          File "/usr/lib/python3.12/site-packages/tmt/base.py", line 3556, in go
            plan.go()
          File "/usr/lib/python3.12/site-packages/tmt/base.py", line 2289, in go
            step.go()
          File "/usr/lib/python3.12/site-packages/tmt/steps/discover/__init__.py", line 345, in go
            phase.go()
          File "/usr/lib/python3.12/site-packages/tmt/steps/discover/fmf.py", line 475, in go
            self.do_the_discovery(path)
          File "/usr/lib/python3.12/site-packages/tmt/steps/discover/fmf.py", line 619, in do_the_discovery
            test.require, test.recommend, _ = tmt.libraries.dependencies(
                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
          File "/usr/lib/python3.12/site-packages/tmt/libraries/__init__.py", line 158, in dependencies
            library = library_factory(
                      ^^^^^^^^^^^^^^^^
          File "/usr/lib/python3.12/site-packages/tmt/libraries/__init__.py", line 109, in library_factory
            library.fetch()
          File "/usr/lib/python3.12/site-packages/tmt/libraries/beakerlib.py", line 326, in fetch
            self._merge_metadata(library_path, local_library_path)
          File "/usr/lib/python3.12/site-packages/tmt/libraries/beakerlib.py", line 202, in _merge_metadata
            data=tmt.utils.get_full_metadata(library_path, self.name),
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          File "/usr/lib/python3.12/site-packages/tmt/utils.py", line 2581, in get_full_metadata
            return fmf.Tree(fmf_tree_path).find(node_path).data
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        AttributeError: 'NoneType' object has no attribute 'data'

@lukaszachy
Copy link
Collaborator Author

How it will be

/var/tmp/tmt/run-296

/default/plan
    discover
        how: fmf
        directory: /tmp/r_171158_diE

plan failed

The exception was caused by 1 earlier exceptions

Cause number 1:

    Can't find library @ name: /Library/basic, url: https://pkgs.devel.redhat.com/git/tests/vsftpd, ref: master.

    The exception was caused by 1 earlier exceptions

    Cause number 1:

        '/Library/basic' not found in the '/var/tmp/tmt/run-296/default/plan/discover/default-0/tmpl12iouq2/pkgs.devel.redhat.com/vsftpd/Library/basic' Tree.

@lukaszachy lukaszachy added this to the 1.33 milestone Apr 9, 2024
@LecrisUT
Copy link
Contributor

In the testing suite is it possible to catch-all any traceback? It would be nice to make sure all expected failures produce a useful message, while a traceback would mean tmt did not expect that to happen.

@happz happz added the trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix. label Apr 30, 2024
Copy link
Collaborator

@thrix thrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add some tests for this?

@psss
Copy link
Collaborator

psss commented May 3, 2024

Would it be possible to add some tests for this?

Yeah, that would be nice. @lukaszachy, any simple way to extend the existing test coverage? I quickly tried to reproduce the problem but I end up with the following error instead:

After beakerlib processing, tests may have only simple requirements

@lukaszachy
Copy link
Collaborator Author

Finally reproduced again: repo with fmf root, directory exists as well but the 'main.fmf' of the library node isn't there.

@lukaszachy
Copy link
Collaborator Author

When adding the test I realized that 'Library @ name' isn't the best way to report the problem.
I've changed it to

plan failed

The exception was caused by 1 earlier exceptions

Cause number 1:

    Library with fmf_id='name: /dir-without-fmf, url: https://github.com/teemtee/tests, ref: missing-node-data, path: /nested' doesn't exist.

    The exception was caused by 1 earlier exceptions

    Cause number 1:

        '/dir-without-fmf' not found in the '/var/tmp/tmt/run-057/plan/certificate/missing/node-metadata/discover/default-0/tmp_5e3wcv3/github.com/tests/nested/dir-without-fmf' Tree.

@lukaszachy
Copy link
Collaborator Author

Once teemtee/tests#7 is merged I can/will have to drop the 'ref' from the test.

@lukaszachy lukaszachy requested a review from thrix May 3, 2024 16:09
@lukaszachy lukaszachy added the full test Pull request is ready for the full test execution label May 3, 2024
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for improving this. Just one typo.

tests/discover/libraries/data/certificate.fmf Outdated Show resolved Hide resolved
When /foo doesn't exist in the fmf root tmt should provide better error
than saying `AttributeError: 'NoneType' object has no attribute 'data'`
@psss psss self-assigned this May 6, 2024
@psss
Copy link
Collaborator

psss commented May 6, 2024

Test failure irrelevant --> merging.

@psss psss merged commit 6b0ebd1 into main May 6, 2024
19 of 20 checks passed
@psss psss deleted the better-error-no-node branch May 6, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full test Pull request is ready for the full test execution libraries trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants