[1.29.30] CI on GitHub Actions#3224
Merged
jirihnidek merged 26 commits intosubscription-manager-1.29.30from Mar 9, 2023
Merged
[1.29.30] CI on GitHub Actions#3224jirihnidek merged 26 commits intosubscription-manager-1.29.30from
jirihnidek merged 26 commits intosubscription-manager-1.29.30from
Conversation
* Card ID: ENT-5317 We've seen some strange issues when the full test suite has been run several times in a row. A process responsible for creating DBus session was not being destroyed *every time* -- when it wasn't, the next DBus test would try to claim the same DBus address again. That would raise an exception in child thread (that was not handled), pytest would get stuck until the timeout happened; the main thread was still asleep, resulting in INTERNALERRORs. This new approach addresses this in several ways: - DBus server is created for whole DBus class, not for every one of the tests. That means that even if the probability of hanging up stayed here, the chance of that happening decreases by an order of magnitude. - The tests aren't using DBus to pass data around now. While the server is needed to instantiate the DBus classes, we can use Python magic to access original functions that were decorated and call them instead, circumventing all DBus communication. This has a side effect of speeding up the tests, as both server creation and DBus transmission added non-zero delay to each of DBus tests. (Cherry-picked from 2e5dd64)
* Card ID: ENT-5317 (Cherry-picked from de9730e)
(Cherry-picked from f368d69)
* Card ID: ENT-5317 (Cherry-picked from 2472cfc)
* Card ID: ENT-5317 (Cherry-picked from fec8833)
* Card ID: ENT-5317 (Cherry-picked from eaac363)
* Card ID: ENT-5317 (Cherry-picked from 5e4d77c)
* Card ID: ENT-5317 (Cherry-picked from 521cf61)
(Cherry-picked from 7324c28)
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)
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)
* 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)
* Card ID: ENT-3759 (Cherry-picked from 124db8c. It does not include changes of test/rhsmlib/dbus/test_facts.py.)
* Card ID: ENT-5541 Publish coverage report as a comment under the PR. - To send the comment only once, not for every matrix system, only 'Fedora latest' is used as a data source. - Because of how GitHub permissions work, the comment is only sent if the PR originates from a feature branch; nothing will be sent for a PR originating from some fork. Full coverage is still displayed in CI output, it is just not sent as a comment. - Package 'pytest-cov' is used instead of 'coverage'. It is still using coverage in a background, but it runs it as a pytest addon instead, allowing us to pass its arguments into pytest, instead of wrapping whole pytest call inside of a coverage invocation. (Cherry-picked from ea495a6)
* Card ID: ENT-5543 (Cherry-picked from 9fa7469)
* Card ID: ENT-5542 (Cherry-picked from 2791a52)
When the package is not available, one test from test/test_dnf_content_plugin.py is skipped. (Cherry-picked from a894837)
They were required because of recent bug in systemd: - https://bugzilla.redhat.com/show_bug.cgi?id=2167468 - systemd/systemd#26366 With this fix, we can remove the test deselections. (Cherry-picked from 8f20aad)
- Rename argument 'auto_spec' to 'autospec' It was valid in older versions of the 'mock' library. (Partially cherry-picked from f6c4404)
This could help us with debugging errors when a test crashes on CI, but not locally. (Cherry-picked from f636251)
The markers were removed before, but the D-Bus tests tend to fail *sometimes*, so it is useful to keep this in. All D-Bus tests inherit from this class, so we only have to do it here in one place. (Cherry-picked from 24e47e4)
Our test that checks if it is possible to connect to newly created socket started failing with 'Permission denied' when the code tried to use it. Pytest cannot write into '/run', moving it to '/tmp' solves the issue. Additionally, as a result of relatively recent D-Bus implementation changes, 'tmpdir' and 'dir' behave the same and create regular socket, not an abstract socket. Connecting with "\0" prefix (which signifies the abstract one) now fails and thus has been removed from the tests. (Cherry-picked from bae7ce3)
This ensures all the injections have been initiated. Without them, the tests may fail on attempts to write '/etc/pki/product' or similar root directories. (Cherry-picked from 729b4f8)
This small patch ensures all injections have been initialized before running the test suite. Without them, inheriting from SubManFixture would have no effect. (Cherry-picked from 4f7ae98)
cnsnyder
requested review from
a team and
ptoscano
and removed request for
a team
findProductId() returns an int, either -1 or 1 depending on whether it was possible to find the product ID in the specified certificate. The problem with this kind of return value is that's very easy to mistake for a boolean, and indeed installProductId() has a boolean-like check that it is always true, no matter the return value of findProductId(). As a fix, change the return type of findProductId() to gboolean, with true returned whether the search succeeded: - reduce the scope of the 'ret_val' variable, so there are less chances to use it wrongly - make 'ret_val' false by default, only changing it to true in the only place where it needs to be like that - adapt the tests accordingly (Cherry-picked from 77aee7e)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a backport of many commits that were required to make the move. Most of the commits applied cleanly; a few did not and were backported only partially, see the commit messages for backport notes.
Original PRs into main, that were sources for the backported commits, are:
findProductId()#3208