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

Backport pytest migration to rhel-9 #3664

Merged
merged 22 commits into from
Dec 7, 2021

Conversation

jkonecny12
Copy link
Member

In this big change I'm backporting and fixing code from upstream. It's transition from nosetests -> unittests -> pytest. Sorry for this being so big.

This PR is backport of:
#3445
#3553
#3473
#3567

To make this works, first it needs to be rebased on top of #3661.

Also we need to decide if want to first merge pending changes to rhel-9 branch and do the changes on the branch or opposite.

@jkonecny12 jkonecny12 added infrastructure Changes affecting mostly infrastructure rhel-9 labels Oct 21, 2021
Copy link
Contributor

@VladimirSlavik VladimirSlavik 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 to me. Thank you!

pyanaconda/core/util.py Outdated Show resolved Hide resolved
@M4rtinK
Copy link
Contributor

M4rtinK commented Oct 25, 2021

Also we need to decide if want to first merge pending changes to rhel-9 branch and do the changes on the branch or opposite.

I recommend waiting at least for #3651 to be merged as that PR has quite a few changes in tests and is already on ready-to-merge state.

Copy link
Contributor

@rvykydal rvykydal 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 to me.

@VladimirSlavik
Copy link
Contributor

It could be good to add also #3608 which is one commit.

Copy link
Contributor

@poncovka poncovka 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 to me. Thank you!

tests/glade/run_glade_tests.py Outdated Show resolved Hide resolved
@M4rtinK
Copy link
Contributor

M4rtinK commented Nov 8, 2021

PR #3651 has been merged, so its not longer blocking pytest migration on RHEL 9.

@jkonecny12
Copy link
Member Author

Yes, I know. I just did not had time to finish this but planning to look on that soon.

@jkonecny12 jkonecny12 force-pushed the rhel-9-backport-pytest branch 2 times, most recently from 6f11b87 to 6984ba0 Compare November 30, 2021 10:14
@jkonecny12
Copy link
Member Author

jkonecny12 commented Nov 30, 2021

UPDATED

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Perhaps you also need #3608 which is just a6cfb29.

@jkonecny12
Copy link
Member Author

UPDATED:
Added #3730 .

@jkonecny12
Copy link
Member Author

jkonecny12 commented Dec 1, 2021

UPDATED:

@jkonecny12
Copy link
Member Author

/kickstart-test --testtype smoke

Python3 unittests need to have `test_*` prefix otherwise they are ignored. On the
other hand nosetest took `test_*`, `*_test.py` or even `*_tests.py`.

(cherry picked from commit 3fbff2c)
This make the tests discoverable by python3 unittest framework.

(cherry picked from commit e0c1928)
We need to migrate away from nosetests because they are not compatible with
python 3.10. Let's finally do the step.

(cherry picked from commit 0bf0aa7)
First we are running these by unittest python3 framework and second this name
could stay even in future when we migrate away from the unittest framework.

Fix import paths in tests too.

(cherry picked from commit 2306137)
Better name in general. Not referring nose framework which we don't use now
anymore.

Remove nosetests_root.sh because the tests it's pointing does not exists.

This is breaking change but it shouldn't be such a pain because I don't think
there are many users (other than from terminal) who use the
`UNIT_TESTS_ARGS` Makefile var or the `tests-unit-only` Makefile target.

`UNIT_TESTS_ARGS` now pass arguments to the `unittest discover` python module.
Change documentation to reflect that.

(cherry picked from commit df1c352)
…fra)

Rename it to UNIT_TESTS_PATTERN and pass this argument to the
`python3 -m unittest discover -k` parameter.

(cherry picked from commit 769efda)
Show how to use the new UNIT_TESTS_PATTERN variable and fix name changes.

(cherry picked from commit 498fa42)
…fra)

The unittest framework is matching `test*` files.

(cherry picked from commit d701c61)
We used nose tests but it doesn't work anymore on python 3.10. Let's finally
migrate away.

Add future TODO to solve RPM_TESTS_ARGS parameter.

(cherry picked from commit 094a950)
Having 'test' keyword in the middle was fine for nosetests but not for unittests
and I don't blame them.
VladimirSlavik and others added 12 commits December 2, 2021 15:00
- Convert from nose to unittest
- Launch via shell script, same as unit and rpm tests
- Remove unused capabilities:
  - No options can't be passed to this test any more
  - Do not consider translations
  - No longer test only specific files
- Simplify format and layout of the individual tests and helper code:
  - Make glade test common code a module, no class needed any more
  - Merge test classes where applicable
- Rename directory

(cherry picked from commit 69e17f5)
All the tests seems to be working as they should and the error reporting is 100%
better than with unit-tests.

(cherry picked from commit a473b59)
During this some of code bad habits were discovered.

old bad habbit -> pytest

self.assertEquals(value, None) -> assert value is None

Also the output is a bit smaller so some test code could be moved to one line.

(cherry picked from commit 2139937)
We don't need this with pytest. All the asserts in pytest are separated statements.

(cherry picked from commit 16260f5)
On a few places in the code we used:

self.assertRaises(Exception, msg="message of exception"):

However, this is not a valid usage. The `msg` argument here is replacement of
the error message raised from the assert, not a message of the exception to
compare.

The conversion script to pytest dropped these `msg` arguments (probably because
pytest doesn't support them). So I'm fixing these tests now.

(cherry picked from commit 4abaf2d)
Sometimes we compare large data structures and we need to see full diff.

(cherry picked from commit 73cb6e9)
Don't propagate these warnings to the unit tests summary.

(cherry picked from commit aa6ef21)
Use '==' instead of 'is' to resolve the following warning:
SyntaxWarning: "is" with a literal. Did you mean "=="?

(cherry picked from commit 837f22b)
The __pycache__ folder is created during local run of the code and it shouldn't
be taken as a test issue.
The assert introspection works by default only for test modules. It means that
helper functions from other modules will raise the AssertionError exception
without any other useful information. These modules have to be explicitly
registered with the register_assert_rewrite function to make the assert
introspection work.

See: https://docs.pytest.org/en/6.2.x/writing_plugins.html#assertion-rewriting

(cherry picked from commit 17a3eb7)
These were previously changed in bulk after switch to pytest.

(cherry picked from commit a6cfb29)
@jkonecny12
Copy link
Member Author

UPDATED:

  • Rebase to solve conflict.

@jkonecny12
Copy link
Member Author

/kickstart-test --testtype smoke

@jkonecny12 jkonecny12 merged commit 7f59099 into rhinstaller:rhel-9 Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Changes affecting mostly infrastructure rhel-9
6 participants