Skip to content

[1.28] ENT-3759: Test on GitHub Actions#3195

Merged
ptoscano merged 11 commits intosubscription-manager-1.28from
mhorky/ENT-2759_actions_1.28
Feb 2, 2023
Merged

[1.28] ENT-3759: Test on GitHub Actions#3195
ptoscano merged 11 commits intosubscription-manager-1.28from
mhorky/ENT-2759_actions_1.28

Conversation

@m-horky
Copy link
Contributor

@m-horky m-horky commented Jan 19, 2023

  • Card ID: ENT-3759

"Backport" of #3192. Because the branches differ in a significant way, it was required to backport some of the commits from main branch fixing test issues.

@m-horky m-horky force-pushed the mhorky/ENT-2759_actions_1.28 branch 2 times, most recently from e1d9f43 to 79a1921 Compare January 19, 2023 12:26
@m-horky m-horky marked this pull request as ready for review January 19, 2023 12:34
@m-horky m-horky marked this pull request as draft January 25, 2023 15:22
When tests were run in container, without having sub-man packages
installed, two things happened:

- Container detection kicked in and prevented CPProvider from being
  created.
  NOTE: rhsm/unit/test_config.py::InContainerTests do not inherit from
  fixture.py::SubManFixture and are not affected by this patch.

- TCP handshake was attempted by opening a connection. Because the
  connection requires RHSM certificates to be installed on the system,
  this step crashed, when subscription-manager-rhsm-certificates (e.g.,
  /etc/rhsm/ca/ directory) was not present.

(Cherry-picked from d1b716f)
@m-horky m-horky force-pushed the mhorky/ENT-2759_actions_1.28 branch from 79a1921 to 805f51a Compare January 31, 2023 15:37
@m-horky m-horky marked this pull request as ready for review January 31, 2023 15:52
Copy link
Contributor

@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, luckly almost all the issues were backports

  • wrt the commit Lock pytest version:

    • I guess you can replace it with f801b69
  • wrt the commit Fix tests for Python 3.6 runner:

    • it mentions changing test-requirements.txt, however it does not do that
    • wouldn't backporting 39e1763 be easier?
    • ditto for the removal of standalone mock, i.e. 7fe24e3
  • wrt the commit ENT-3759: Test on GitHub Actions

    • I think we can drop centos9 stream here, as 1.28 won't reach that version
    • I'd drop Fedora Rawhide, I don't think we want a fast-changing Fedora to break a stable branch

This makes --randomly-seed useless, because previously failed tests are
run first instead.

(Cherry-picked from e3ea1e4)
@m-horky
Copy link
Contributor Author

m-horky commented Feb 2, 2023

1/ True, I'll backport f801b69 instead
2a/ I considered backporting 39e1763, but which is still used in 1.28 (in kpatch.py) and I did not want to touch it.
2b/ 7fe24e3 requires manual touches, as there were some incompatible changes in the meantime. Will do.
3/ I know Rawhide and cs9 won't reach that version, but my reasoning was that we still need to maintain that code on current Fedora versions. By catching hard incompatibilities we can ensure that we can still run the code while backporting patches from main.

@ptoscano
Copy link
Contributor

ptoscano commented Feb 2, 2023

2a/ I considered backporting 39e1763, but which is still used in 1.28 (in kpatch.py) and I did not want to touch it.

Oh there's the parent commit of that that switches away from the custom which implementation: 1560e82 (see #2898).

@m-horky m-horky force-pushed the mhorky/ENT-2759_actions_1.28 branch 2 times, most recently from dd04736 to 978e956 Compare February 2, 2023 11:31
m-horky and others added 9 commits February 2, 2023 13:29
- Stop using .addClassCleanup().
  The function was added to unittest in Python 3.8, but this branch
  requires Python 3.6.

- Fix D-Bus test of Consumer object.
  Patch was being stopped incorrectly.

- Fix collector test for aarch64 uuid.
  The test was not setting up mocks properly and a real value was read.
  This made the test work on development machines but fail on GitHub
  Actions runner.

- Rename argument 'auto_spec' to 'autospec'
  It was valid in older versions of the 'mock' library.
Python 3.3 provides it, so switch to it since the required Python
version is 3.6.

(Cherry-picked from 1560e82)
It is no more used now, so drop it along with its tests.
It seems like pytest 7 breaks pytest-forked somehow, as also reported in
pytest-dev/pytest#9621

Hence, for now pin it to any version lower than 7 to keep the unit tests
working.

(Cherry-picked from f801b69)
Several tests fail when 'pytest' is run under root, even if they work
correctly when run as non-root user. By adding the new pytest marker
we can ensure that these tests are skipped, instead of raising false
alarms.

(Cherry-picked from c4bf220)
This test used to do some comprehensive testing of the translations,
long time ago. Nowadays, it does few things:
1) tries to translate a certain message in some languages
2) checks that the translation of a different message is unicode

First of all: these tests load translations from the system, so they
should be marked as functional.

Second: gettext() returns the same string in case the string is not
translated in the current locale, so in some cases the original strings
are checked to be unicode, which they obviously are.

Third: gettext (and Weblate, which is currently used to manage
translations) already do a good job in checking for wrongly encoded
messages, so there is no need to do the same check locally.

Hence, simply drop test_po_files.py, as there is nothing useful in it.

Card ID: ENT-4484

(Cherry-picked from e9cb4bc)
* Card ID: ENT-5536

Subprocess calls to 'touch' were sometimes failing to update the
modification date (race condition, probably?), making the tests fail.
When 'os.utime' is used, this is no longer the case and the files are
always reported as modified.

(Cherry-picked from 68acefc)
Because 'get_config_parser()' and 'in_container()' functions were being
imported directly, instead of as reference, the mocks that tests perform
were not applying to them, causing test suite to fail when run from
inside a container. Future-wise it is better to manage mocks on one
place, instead of patching the functions whenever they are being
imported.

(Cherry-picked from 0b78761)
@m-horky m-horky force-pushed the mhorky/ENT-2759_actions_1.28 branch from 978e956 to 17e72fe Compare February 2, 2023 12:30
Copy link
Contributor

@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

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

\o/

@ptoscano ptoscano merged commit 70b3c2e into subscription-manager-1.28 Feb 2, 2023
@ptoscano ptoscano deleted the mhorky/ENT-2759_actions_1.28 branch February 2, 2023 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants