Use python imports to identify fixtures (part 4/6) #5704

cognifloyd · 2022-08-11T16:21:40Z

Background

I'm working towards introducing pants. Eventually I would like to use pants to run tests to take advantage of the fine-grained per-file caching of results that accounts for dependencies by python files (pants infers the deps by reading the python files).

In order to use the fine-grained caching, Pants needs to know which tests rely on which fixtures. We could add extra metadata to tie the tests to the fixtures, but that would be an additional maintenance burden that will become out-of-date. We can also include all fixtures for all tests, but then we don't benefit from the fine-grained per-file caching. However, pants can already infer dependencies based on python imports, so that is what this PR (and several follow-up PRs) takes advantage of.

Overview

This PR does the following:

turn every fixture into an importable python module with an __init__.py.
add a fixture.py file in each fixture that uses the fixturesloader utils (where helpful) to identify itself with PATH and NAME vars.
also add a DIR_NAME var for the fixture packs that use a name that differs from the dir name
in every test that uses a given fixture, import PATH, NAME and/or DIR_NAME vars from that fixture.

This PR focuses on only fixture packs that needed some custom bits in fixture.py like the DIR_NAME var.
Follow-up PRs will address other fixture packs and other sets of fixtures. I will submit those PRs after this one is merged.

Statistics

Changes Summary:

1 line updated in changelog
5 copies of fixture.py (~16 lines, 13 of which are copyright/license header): 18+17*3+16=+85 lines
5 fixture.py files adjusted (improve vars for special pack fixtures): (-1 +2)*5=(-5 +10)
-47 +52 lines changed in test files to use the new fixture vars.

rush-skills

LGTM Overall, had a minor question below

rush-skills · 2022-08-12T16:09:25Z

st2api/tests/unit/controllers/v1/test_packs_views.py

-        self.assertEqual(len(resp.json), 1)
-        self.assertEqual(resp.json[0]["file_path"], "pack.yaml")
+        self.assertEqual(len(resp.json), 3)
+        self.assertIn("pack.yaml", [f["file_path"] for f in resp.json])


What does this change?

The pack in question only had pack.yaml before this PR. I added fixture.py and __init__.py, so the file count goes up to 3.
And, since there is more than one file, we can't just assume that the resp.json[0] is the pack.yaml file. But is it [1] or [2]? It's hard to say because the order might depend on the the filesystem. I tried resp.json[2] which on GHA was not pack.yaml, so they are not sorted alphabetically. To avoid a flaky test relying on a particular sort, I made a list of the files with [f["file_path"] for f in resp.json], and just assert that pack.yaml is in it.

cognifloyd · 2022-08-17T16:54:57Z

st2tests/st2tests/fixtures/packs/test_content_version_fixture/fixture.py

+PACK_NAME = "test_content_version"
+_, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)
+PACK_PATH = PACK_PATH[: -len("_fixture")]


The test_content_version fixture is in a git submodule. I wanted to keep all the fixture details in this repo so that they're available even if someone has not cloned the submodule. So, I put the fixture vars in test_content_version_fixture directory and adjusted the vars.

cognifloyd

Here are the sources for many of the pack fixture changes. (FYI - for anyone needs to reference this in the future)

cognifloyd · 2022-08-17T16:58:02Z

st2tests/st2tests/fixtures/packs/dummy_pack_10/fixture.py

+PACK_NAME = "dummy_pack_10_wrong_deps"
+PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)


st2/st2tests/st2tests/fixtures/packs/dummy_pack_10/pack.yaml

Line 2 in 6db0561

name : dummy_pack_10_wrong_deps

cognifloyd · 2022-08-17T16:58:31Z

st2tests/st2tests/fixtures/packs/dummy_pack_14/fixture.py

+PACK_NAME = "dummy pack 14"
+PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)


st2/st2tests/st2tests/fixtures/packs/dummy_pack_14/pack.yaml

Line 2 in 6db0561

name : dummy pack 14

cognifloyd · 2022-08-17T17:01:23Z

st2tests/st2tests/fixtures/packs/dummy_pack_16/fixture.py

+# limitations under the License.
+from st2tests import fixturesloader
+
+PACK_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)


This one did not need special vars. Instead, it needed special handling in one of the tests that counted the number of files in the pack.

cognifloyd · 2022-08-17T17:01:55Z

st2tests/st2tests/fixtures/packs/dummy_pack_18/fixture.py

+PACK_NAME = "Dummy Pack 18"
+PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)


st2/st2tests/st2tests/fixtures/packs/dummy_pack_18/pack.yaml

Line 2 in 6db0561

name : Dummy Pack 18

cognifloyd · 2022-08-17T17:02:16Z

st2tests/st2tests/fixtures/packs/dummy_pack_7/fixture.py

+PACK_NAME = "dummy_pack_7_name"
+PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)


st2/st2tests/st2tests/fixtures/packs/dummy_pack_7/pack.yaml

Line 2 in 6db0561

name : dummy_pack_7_name

cognifloyd · 2022-08-17T17:02:37Z

st2tests/st2tests/fixtures/packs/dummy_pack_8/fixture.py

+PACK_NAME = "Dummy Pack 8 Invalid Name"
+PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)


st2/st2tests/st2tests/fixtures/packs/dummy_pack_8/pack.yaml

Line 2 in 6db0561

name : Dummy Pack 8 Invalid Name

cognifloyd · 2022-08-17T17:02:54Z

st2tests/st2tests/fixtures/packs/dummy_pack_9/fixture.py

+PACK_NAME = "dummy_pack_9_deps"
+PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)


st2/st2tests/st2tests/fixtures/packs/dummy_pack_9/pack.yaml

Line 2 in 6db0561

name : dummy_pack_9_deps

cognifloyd · 2022-08-17T17:03:25Z

st2tests/st2tests/fixtures/packs/pack_dir_name_doesnt_match_ref/fixture.py

+PACK_NAME = "pack_name_not_the_same_as_dir_name"
+PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)


st2/st2tests/st2tests/fixtures/packs/pack_dir_name_doesnt_match_ref/pack.yaml

Lines 1 to 2 in 6db0561

ref: pack_name_not_the_same_as_dir_name

name: pack_name_not_the_same_as_dir_name

cognifloyd added this to the 3.8.0 milestone Aug 11, 2022

cognifloyd added enhancement maintenance refactor tests labels Aug 11, 2022

cognifloyd self-assigned this Aug 11, 2022

pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Aug 11, 2022

cognifloyd mentioned this pull request Aug 11, 2022

Use python imports to identify fixtures (part 1/6) #5699

Merged

cognifloyd changed the base branch from fixtures-loading-3 to master August 11, 2022 17:23

pull-request-size bot added size/XXL PR that changes 1000+ lines. You should absolutely split your PR into several. and removed size/L PR that changes 100-499 lines. Requires some effort to review. labels Aug 11, 2022

cognifloyd force-pushed the fixtures-loading-4 branch 3 times, most recently from 769d0c8 to e7135b0 Compare August 11, 2022 18:55

cognifloyd changed the base branch from master to fixtures-loading-3 August 11, 2022 19:06

pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/XXL PR that changes 1000+ lines. You should absolutely split your PR into several. labels Aug 11, 2022

cognifloyd force-pushed the fixtures-loading-4 branch from e7135b0 to 8bb4b2b Compare August 11, 2022 19:43

cognifloyd force-pushed the fixtures-loading-3 branch from 30c2b0e to a13542a Compare August 11, 2022 19:52

cognifloyd force-pushed the fixtures-loading-4 branch from 8bb4b2b to 417794e Compare August 11, 2022 19:53

cognifloyd changed the base branch from fixtures-loading-3 to master August 11, 2022 19:53

pull-request-size bot added size/XXL PR that changes 1000+ lines. You should absolutely split your PR into several. and removed size/L PR that changes 100-499 lines. Requires some effort to review. labels Aug 11, 2022

cognifloyd force-pushed the fixtures-loading-4 branch from 417794e to f4a651b Compare August 11, 2022 19:54

cognifloyd changed the base branch from master to fixtures-loading-3 August 11, 2022 20:40

pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/XXL PR that changes 1000+ lines. You should absolutely split your PR into several. labels Aug 11, 2022

cognifloyd changed the title ~~Use python imports to identify fixtures (part 4)~~ Use python imports to identify fixtures (part 4/6) Aug 11, 2022

cognifloyd added the pantsbuild label Aug 11, 2022

cognifloyd requested a review from rush-skills August 12, 2022 15:48

rush-skills approved these changes Aug 12, 2022

View reviewed changes

cognifloyd force-pushed the fixtures-loading-3 branch from a13542a to 216129e Compare August 17, 2022 15:22

cognifloyd force-pushed the fixtures-loading-4 branch from f4a651b to fba1c80 Compare August 17, 2022 16:22

Base automatically changed from fixtures-loading-3 to master August 17, 2022 16:31

cognifloyd added 4 commits August 17, 2022 11:33

Use python imports to identify fixtures (part 4)

c0a624f

update changelog

e9714a2

Use python imports to identify test_content_version fixture pack

ec78bef

adjust file count for test that uses dummy_pack_16

8283904

cognifloyd force-pushed the fixtures-loading-4 branch from fba1c80 to 8283904 Compare August 17, 2022 16:33

cognifloyd marked this pull request as ready for review August 17, 2022 16:33

cognifloyd requested a review from a team August 17, 2022 16:34

nzlosh approved these changes Aug 17, 2022

View reviewed changes

cognifloyd commented Aug 17, 2022

View reviewed changes

cognifloyd enabled auto-merge August 17, 2022 17:06

cognifloyd merged commit 8acbd31 into master Aug 17, 2022

cognifloyd deleted the fixtures-loading-4 branch August 17, 2022 17:13

cognifloyd mentioned this pull request Jan 27, 2023

Tell pants about tests that use globs to test with many pack fixtures #5874

Merged

cognifloyd mentioned this pull request Sep 14, 2024

Pants: Use python imports to identify fixtures #6240

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use python imports to identify fixtures (part 4/6) #5704

Use python imports to identify fixtures (part 4/6) #5704

cognifloyd commented Aug 11, 2022 •

edited

Loading

rush-skills left a comment •

edited

Loading

rush-skills Aug 12, 2022

cognifloyd Aug 12, 2022

cognifloyd Aug 17, 2022

cognifloyd left a comment •

edited

Loading

cognifloyd Aug 17, 2022

cognifloyd Aug 17, 2022

cognifloyd Aug 17, 2022

cognifloyd Aug 17, 2022

cognifloyd Aug 17, 2022

cognifloyd Aug 17, 2022

cognifloyd Aug 17, 2022

cognifloyd Aug 17, 2022

		PACK_NAME = "dummy_pack_10_wrong_deps"
		PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)

		PACK_NAME = "dummy pack 14"
		PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)

		PACK_NAME = "Dummy Pack 18"
		PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)

		PACK_NAME = "dummy_pack_7_name"
		PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)

		PACK_NAME = "Dummy Pack 8 Invalid Name"
		PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)

		PACK_NAME = "dummy_pack_9_deps"
		PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)

		PACK_NAME = "pack_name_not_the_same_as_dir_name"
		PACK_DIR_NAME, PACK_PATH = fixturesloader.get_fixture_name_and_path(__file__)

	ref: pack_name_not_the_same_as_dir_name
	name: pack_name_not_the_same_as_dir_name

Use python imports to identify fixtures (part 4/6) #5704

Use python imports to identify fixtures (part 4/6) #5704

Conversation

cognifloyd commented Aug 11, 2022 • edited Loading

Background

Overview

Statistics

rush-skills left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cognifloyd left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cognifloyd commented Aug 11, 2022 •

edited

Loading

rush-skills left a comment •

edited

Loading

cognifloyd left a comment •

edited

Loading