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

Export support --bugzilla to link case #808

Merged
merged 2 commits into from
Sep 2, 2021
Merged

Export support --bugzilla to link case #808

merged 2 commits into from
Sep 2, 2021

Conversation

lukaszachy
Copy link
Collaborator

@lukaszachy lukaszachy commented Jun 17, 2021

Todo:

  • Error handling (permissions, login,...)
  • Test coverage

All done, ready for final review

@lgtm-com
Copy link

lgtm-com bot commented Jun 17, 2021

This pull request introduces 1 alert when merging d129af2 into 377dd6b - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lukaszachy lukaszachy added export needs tests Test coverage to be added for the affected code labels Jun 18, 2021
@lukaszachy
Copy link
Collaborator Author

The 'export' part should be ready. Asking for your feedback.

@lukaszachy lukaszachy linked an issue Jun 18, 2021 that may be closed by this pull request
@lgtm-com
Copy link

lgtm-com bot commented Jun 18, 2021

This pull request introduces 3 alerts when merging 5ea421c into 377dd6b - view on LGTM.com

new alerts:

  • 2 for Wrong name for an argument in a call
  • 1 for Unused local variable

@lukaszachy lukaszachy marked this pull request as ready for review June 18, 2021 11:40
@lukaszachy lukaszachy added this to the 1.7 milestone Jun 18, 2021
@lukaszachy lukaszachy added enhancement New feature or request and removed needs tests Test coverage to be added for the affected code labels Jun 22, 2021
@lukaszachy
Copy link
Collaborator Author

/packit test

@psss
Copy link
Collaborator

psss commented Jul 14, 2021

@hegerj, perhaps something you would like to have a look at?

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.

Thanks for implementing this! Looks good and works as expected. Added a short mention about the new feature to the documentation plus a couple of minor adjustments in aff7103. Please, have a look if everything's ok.

@psss psss self-assigned this Aug 17, 2021
@lukaszachy
Copy link
Collaborator Author

@psss LGTM, thanks for adjustments.

@psss
Copy link
Collaborator

psss commented Aug 17, 2021

Good, thanks. The integration test seems to be failing. Could you please have a look?

@psss
Copy link
Collaborator

psss commented Aug 17, 2021

Tests are failing in a weird way, seems we need a requre expert. @jscotka, could you please have a look what could be wrong? The newly added test passes but other two existing are now failing.

@psss psss modified the milestones: 1.7, 1.8 Aug 17, 2021
@lukaszachy
Copy link
Collaborator Author

What I found so far:

  • if newly added test doesn't run tmt (--> require is not used there) or is disabled then all test pass
  • moving newly added test to own class didn't help

test_create fails with unable to create a test case error which is raised as returned dict (from require's tcms call) doesn't contain case_id (which would be if case was created). But I don't see how this could happen. test_create require playbook was not touched at all.

@jscotka
Copy link
Collaborator

jscotka commented Aug 20, 2021

there is correlation between nitrate cache and order of test case execution inside pytest.
There is more ways how to fix this:

  1. disable nitrate cache and rut without caching and regenerate the data
  2. let's do separate tmt cases for each test inside pytest and regenerate
  3. document this issue and just try to let new cases at the end and then regenerate just new one. (but when execute test alone will lead to error.)

@lukaszachy
Copy link
Collaborator Author

I've pushed commit which splits tests/integration execution into two virtual cases. Not ideal but hopefully it will work

@lukaszachy
Copy link
Collaborator Author

fedora-35 issue is caused by sphinx not yet supporting python 3.10 (sphinx-doc/sphinx#9562)

@jscotka
Copy link
Collaborator

jscotka commented Aug 23, 2021

I've pushed commit which splits tests/integration execution into two virtual cases. Not ideal but hopefully it will work

Yep, it is possible solution. better would be to split all testcases, instead of random splittng, because then when you execute some testcases by pytest alone it will fail as well, because now it depends on order.

Or maybe better solution could be to clean nitrate cache after each testcase, then it should be more predicable.
@psss it is possible to clean all nitrate cache on demand e.g. in tearDown ?

@lukaszachy
Copy link
Collaborator Author

Personally I'd prefer to get this PR merged and do the testsuite improvements as a separate effort.
Pipeline is passing.

@psss
Copy link
Collaborator

psss commented Sep 2, 2021

@psss it is possible to clean all nitrate cache on demand e.g. in tearDown ?

Yes, it's possible to call Cache().clear() to clean up the cache content. But I agree with @lukaszachy to address this as a separate pull request.

lukaszachy and others added 2 commits September 2, 2021 10:30
For bugzilla links  with verifies relation in the test:
    Add Bug to the Nitrate case
    Optionally link Bug to the Nitrate case

Add python-bugzilla dependency for tmt-tests-convert
Claim Python 3.9 compatibility in Pypi
Split tests/integration into two

Resolves #689
Add a short mention about the new option to the documentation.
Plus a couple of minor style and wording adjustments.
@psss
Copy link
Collaborator

psss commented Sep 2, 2021

Rebased and squashed, let's see if tests are green.

@psss
Copy link
Collaborator

psss commented Sep 2, 2021

Known failure on Fedora 34, otherwise green.

@psss psss merged commit a979bb0 into master Sep 2, 2021
@psss psss deleted the lzachar-export-bz branch September 2, 2021 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request export
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFE: tmt tests export --nitrate should be able to update bugzilla
3 participants