Seed test fixtures once per module instead of once per test - #2469
Merged
Conversation
Two local fixtures created their nodes over HTTP on every test that used them. Both were declared with a bare @pytest.fixture, so the default function scope re-ran the whole seeding sequence per test -- 6 node creations across 23 tests in the fan-out guard file, 7 tag creations across 10 tests in the tags GraphQL file. Each POST re-parses and re-validates SQL through the full app, so the repeats dominated those modules. Widen both to module scope and point them, and the tests that share their data, at the module-scoped client. Neither module mutates the seeded state -- the fan-out tests only read through /sql/measures/v3/ and the tags tests issue read-only GraphQL queries -- so one seeding pass per module is equivalent. fanout_guard_test.py 122.15s -> 56.14s (23 passed) graphql/tags_test.py 53.64s -> 49.25s (10 passed) Excluding the one-time template build those modules now spend 2.4s and 2.5s on setup, down from 68.3s and ~7s.
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
Same change as the fan-out and tags fixtures: client_with_edge_shapes created four transforms over HTTP on each of its three consuming tests. Nothing in the module mutates them, so one seeding pass per module is equivalent. transform_query_shapes_test.py 56.44s -> 51.79s (3 passed)
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
1 participant
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.
Summary
Some test fixtures are declared using
@pytest.fixturewith no scope, so it defaults to thefunctionscope, which causes it to rerun the entire seeding sequence per test. That seeding sequence will recreate a series of nodes over HTTP, which can get expensive across dozens of tests (e.g., 6 node creations across 23 tests in the fan-out guard file, 7 tag creations across 10 tests in the tags GraphQL file etc).This PR widens them to module scope and then points the tests at the module-scoped client. Since neither of these modules are mutating the seeded state (the fan-out tests only read through
/sql/measures/v3/and the tags tests issue read-only GraphQL queries), one seeding pass per module is sufficient.When run locally:
Test Plan
make checkpassesmake testshows 100% unit test coverageDeployment Plan