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

Run more tests via Pants + pytest #6202

Draft
wants to merge 42 commits into
base: master
Choose a base branch
from
Draft

Run more tests via Pants + pytest #6202

wants to merge 42 commits into from

Conversation

cognifloyd
Copy link
Member

No description provided.

@cognifloyd cognifloyd added this to the pants milestone May 22, 2024
@cognifloyd cognifloyd self-assigned this May 22, 2024
@pull-request-size pull-request-size bot added the size/S PR that changes 10-29 lines. Very easy to review. label May 22, 2024
First, add metadata so pants can load the packs base path fixtures
from st2common/resources for the content laoder tests.
Second, add a fixture.py file in each base path, similar to the
fixtures we have in st2tests, so that pants dependency inferrence
can see where these fixtures are used.
Third, the fixture file ended up creating a __pycache__ directory
in the packs base paths which made tests fail due to an unexpected
extra "pack" named "__pycache__". So, I excluded that as a valid
pack name.
First, add metadata so pants can load the conf fixture from
st2common/tests/resources for the logger test.

Second, add a fixture.py file, similar to the fixtures we have in
st2tests, so that pants dependency inferrence can see where these
fixtures are used.
First, add metadata so pants can load the loadableplugin fixture
from st2common/tests/resources for the plugin loader test.

Second, add a fixture.py file, similar to the fixtures we have in
st2tests, so that pants dependency inferrence can see where these
fixtures are used.
Once pants can build our packages, all of the dist_utils bits should be deleted.
Various tests were relying on the side effects of tests that nosetest
runs before they ran. These include:
- 3 in test_action_alias_utils.py::TestInjectImmutableParameters
- 2 in test_jinja_render_data_filters.py
- 1 in test_logging_middleware.py
- 9 in test_operators.py::SearchOperatorTest
- 3 in test_util_payload.py

In particular, the oslo config initialization from the tests in
st2common/tests/unit/services/ happens before these test ran, obscuring
their dependence on this initialization.

Pants runs each test file separately for fine-grained caching.
@pull-request-size pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/S PR that changes 10-29 lines. Very easy to review. labels May 24, 2024
This looks like copy pasta as some of these files have a comment
saying that running this before importing something else is required.
However, by importing st2tests, that already implicitly happens
in st2tests/st2tests/base.py. Then tests_config.parse_args() gets called
again in the class init.

Plus, I reviewed all the other imports, and none of them have import time
side effects that matter for oslo config bits. So, these calls are not
necessary, and the comments about them are wrong.
pants runs each test file separately. test_workflow_rerun only worked
under nosetest because earlier files already did the monkey_patch.
Without this, running this file in isolation, with either nosetest
or pytest, hangs.
This way we do not need to patch the conf files in so many places.
all are passing locally now
These are not test files:
- st2actions/tests/unit/test_async_runner.py
- st2actions/tests/unit/test_polling_async_runner.py

It looks like they were copied to st2tests/st2tests/mocks/runners/
at some point. Nothing imports from or uses the copies in st2actions,
so just delete them.
…ffects

importing anything form st2tests already handles running
st2tests.config.parse_args() on import before loading
the files from st2common that need those side effects.
So, rely on that, and on the db test case base classes
for running parse_args() where appropriate.

The import side-effects are unfortunate, but this reduces
how many places are making those changes.
…fects

importing anything form st2tests already handles running
st2tests.config.parse_args() on import before loading
the files from st2common that need those side effects.
So, rely on that, and on the db test case base classes
for running parse_args() where appropriate.

The import side-effects are unfortunate, but this reduces
how many places are making those changes.
Actually there are no unit tests in st2tests/, so this just
ensures no one will add one without pants running it.
@pull-request-size pull-request-size bot added size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. and removed size/L PR that changes 100-499 lines. Requires some effort to review. labels May 24, 2024
@pull-request-size pull-request-size bot added size/XXL PR that changes 1000+ lines. You should absolutely split your PR into several. and removed size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. labels May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure: ci/cd no changelog No Changelog.rst needed for this PR pantsbuild size/XXL PR that changes 1000+ lines. You should absolutely split your PR into several. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant