Run pytest on multiple files the pytest-way

[kerys]

pants test triggers pytest on a per-file basis. This has different behavior compared to running pytest <foldername> because each pytest session only knows the single test file. This results in:

• the pytest report summary on terminal is created per test file, and the only really useful test summary is the one provided by pants which provides less information compared to the pytest summary.
• pytest plugins that create summary reports (such as pytest-html, pytest-json) create a single report per file (compared to a single report for all executed tests)
• pytest session scope fixtures are initialized repeatedly because for each test file a new pytest session is started.

I'm just speaking for myself, but I got so used to these points just working when using pytest that running python tests in pants as it is now feels like a downgrade to me. It would be great if there was a way to have the "vanilla" pytest experience also when using pants.

[tseader]

Also feeling the pain with session-wide fixtures while using pytest-bdd. Pants generates a cucumber.json report per file and over-writes the previous one - it's all rather unfortunate.

[kaos]

Related comment #15197 (comment)

[danxmoran]

We are also feeling the pain of per-file (vs. per-directory) pytest runs.

We run django.setup() in a conftest.py near the root of our project. In our pre-Pants CI system that runs pytest <folder> we see this takes a total of 120-150ms per workflow run. In our Pants-based CI, we see it takes a total of ~17640000-23500000ms (~294-392 minutes!!). We split our tests across 16 workers (using PANTS_TEST_SHARD) each with 14cpu and 26gb RAM, and we still see individual shards take upwards of 20 minutes to complete in CI 😞

[danxmoran]

More thoughts: A solution for this would (unfortunately) need to be more complex than a global config toggle. Everything under the django.setup() I mentioned above could be batched together, but we have another directory tree that django.setup()s with different settings in its own conftest.py - these would need to run in separate batches.

Thinking out loud: Could we have a new field on python_test (and maybe other _test targets?) for setting a "batch ID"? If set, targets with the same value are batched into the same pytest process. It could be paired with __defaults__ to batch entire directories at a time.

[Eric-Arellano]

Thinking out loud: Could we have a new field on python_test (and maybe other _test targets?) for setting a "batch ID"? If set, targets with the same value are batched into the same pytest process. It could be paired with defaults to batch entire directories at a time.

I really like an idea like that. What makes me nervous is when Pants tries to get too clever.

@danxmoran with your specific use case, would you still want caching to be at the individual file level? We can run 5 test files in a single batch, and then still cache at the individual file level? If that is not essential for you, this actually becomes pretty easy to implement!

[danxmoran]

@Eric-Arellano per-file caching would be nice, but I think not essential for us in the short-term... all the code in this tree is so intertwined that if any one python_source changes, ~every python_test in the tree gets invalidated. I do think it'd be important for per-file change-detection to still work, though - i.e. if one test in batch "A" is changed I wouldn't want every other test marked with that batch to be immediately marked as changed, too. Not sure how much code reuse there is between the changed subsystem and the caching implementation.

[stuhood]

How would you feel about a global "batch size" setting instead? The test rules could generically batch deterministically into batches of that size across test targets with identical Field values. It's more magical, but explicit batching would be pretty boilerplatey.

[danxmoran]

More magic / less boilerplate sounds good to me 😄 though I'm not fully understanding how I could manually enforce that two groups of tests never run in the same batch with just a batch-size parameter. For example, if I have:

src/
  django_monolith_apps/
    conftest.py
    ...
  projects/
    django_microservice1/
      conftest.py
      ...
    django_microservice2/
      conftest.py
      ...

Everything under src/django_monolith_apps/ can be batched together arbitrarily, and ditto for each of the django_microserviceNs, but it is not safe to batch files across those trees (because their conftests configure differnet test settings). How would that work with just a global "batch size "setting?

[stuhood]

how I could manually enforce that two groups of tests never run in the same batch with just a batch-size parameter.

I'm not sure that you need to. Assuming that pytest properly supports nested/overlapping conftest.py files overriding one another (...?), then batching across the boundary would be safe. Maybe slightly less efficient though, because both configurations would be used, but at different points in the batch run.

But automatically batching seems like it is likely to make back up the performance difference by virtue of not needing manual adjustment.

[danxmoran]

Ah, the need to keep the batches separate comes from Django, not pytest itself. AFAIK django.setup() (which we call in the conftest.pys) can only be called once per process - the 2nd+ calls are no-ops, so you can't swap back and forth between settings / sets of apps.

[danxmoran]

@stuhood @Eric-Arellano is there anything I can do to help push this one forward while you're busy working on the docker-environment support? My first thought is to work through all the test-related types and replace single addresses with collections (i.e. in TestResult here), but leave all the collections as singletons for now. That way the plumbing is in place.

[benjyw]

Ah, the need to keep the batches separate comes from Django, not pytest itself. AFAIK django.setup() (which we call in the conftest.pys) can only be called once per process - the 2nd+ calls are no-ops, so you can't swap back and forth between settings / sets of apps.

I didn't quite understand this. Ignoring Pants, if you run pytest directly on all your tests (across multiple conftests), does the right thing happen, somehow? Or would you have to manually batch the pytest invocations in that case as well?

[danxmoran]

Or would you have to manually batch the pytest invocations in that case as well?

Yes, before Pants we had separate CI jobs/workflows for the different Django projects.

[cedric-fauth]

Hi, I've got similar problems. I am using pants inside a monorepo with hundrets of tests. When running pants test I recognized that each test takes about 7 secs. When using pytest it only takes 200ms. I saw someone else had simmilar problems so I tried the following steps: https://app.slack.com/client/T046T6T8L/C046T6T9U/thread/C046T6T9U-1658183838.164319
I saw that running --setup-only makes nearly no difference in execution time so clearly the setup takes too long and the actual test runs very fast. The output of --setup-only gets stuck at SETUP S django_db_setup (fixtures used: django_db_blocker, django_db_createdb, django_db_keepdb, django_db_modify_db_settings, django_db_use_migrations, django_test_environment) for roughly 6 seconds which is 6/7 of the whole test duration. Running all test in this project would take hours when every tests needs 7-10 secs using pants. With pytest it takes 4 minutes.
pytest.ini

[pytest]
DJANGO_SETTINGS_MODULE = django_core.settings
# -- standard arguments:
addopts:
	--nomigrations
	--create-db
	-vv

pants.toml

[pytest]
lockfile = "lockfiles/python/tools/pytest"
version = "pytest>=7.1.2,<7.2"
extra_requirements.add = [
  "pytest-django==4.5.2",
  "pytest-icdiff==0.5",
  "mixer==7.2.1"
]
config_discovery = true

[benjyw]

Or would you have to manually batch the pytest invocations in that case as well?

Yes, before Pants we had separate CI jobs/workflows for the different Django projects.

Ouch! OK, so that's a wrinkle to take care of.

[stuhood]

@danxmoran's design for this is over here: https://docs.google.com/document/d/1U0Q43bRod_EeVP4eQpcN36NMlxZ4CpRzTl2gMQ5HHvg/edit#

[danxmoran]

Update: I'm working on translating @thejcannon's recently-added pattern of partitioning lint/fmt targets into the test goal. I plan to land a PR refactoring the plumbing but still testing one-module-per-process, then follow up with a pytest-specific PR to implement the new partitioning/batching logic.