dev: Pytest-split test #1144

ajay-sentry · 2025-02-07T21:58:34Z

Purpose/Motivation

This PR tests adding the pytest-split package and running it locally to see if it could potentially help speed up our CI.

On the one hand, it was super straight forward to setup and for our CI it would help a lot with running the tests (if we choose 5 groups for example, each would take ~1-2 minutes to run), and the documentation states that we don't need to run --store-durations all that often, unless our test suite changes a lot since it uses the average test duration for each subsequent test added.

On the other hand, this doesn't help with test runtimes for the full suite locally. While you can be selective with the tests you run, I usually run the full suite initially to see which tests have broken for any PR prior to running individual test suites to fix those tests. If it's found that most folks don't usually run the full suite locally as well then maybe it's not a big deal.

This PR handles everything that's needed from the API side, we'd need to spin up some new GH actions though for each test group that would run after though.

They have a demo repo on their docs with how to set up with a generic test suite

UPDATE:

We got it to work and the results are pretty good! CI times for API look to have gone from ~15:30 -> 6:30, or a 58% reduction in CI run time!

Before run: https://github.com/codecov/codecov-api/actions/runs/13274970819
After run: https://github.com/codecov/codecov-api/actions/runs/13315813179

The test-durations file needs to be committed because it's the thing that pytest-split references when it creates the groups. We can choose to commit it every month or year or anything else, but it's not required outside of the first time since it'll just use the suite's average test runtime for all "new" tests added to the suite.

Again, this doesn't modify anything for your local docker setup either, but you could run the test groups locally if you choose to via pytest --splits "numSplits" --group "groupNum"

Where to go from here?

If there's a way to cache the "bring test env" step up that might be another way to shave off a minute across each runner since those times can range from 20 seconds to ~2 minutes if we get unlucky with network bandwidth for the runner
Continue to move tests from TransactionTestCase -> TestCase as we can, since they're like twice as fast

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

codecov · 2025-02-07T22:12:31Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.07%. Comparing base (a49a632) to head (6e028a8).
Report is 5 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #1144    +/-   ##
========================================
  Coverage   96.07%   96.07%            
========================================
  Files         838      836     -2     
  Lines       19775    20005   +230     
========================================
+ Hits        18998    19220   +222     
- Misses        777      785     +8

Flag	Coverage Δ
unit	`95.93% <ø> (-0.04%)`	⬇️
unit-latest-uploader	`95.93% <ø> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codecov-notifications · 2025-02-07T22:12:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

codecov-qa · 2025-02-07T22:12:35Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.93%. Comparing base (a49a632) to head (6e028a8).
Report is 5 commits behind head on main.

✅ All tests successful. No failed tests found.

github-actions · 2025-02-07T22:12:46Z

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

codecov-public-qa · 2025-02-13T18:01:59Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
456	1	455	0

View the top 1 failed tests by shortest run time

graphql_api/tests/test_pull.py::TestPullRequestList::test_compare_bundle_analysis_missing_reports

Stack Traces | 0.321s run time

self = <django.db.backends.utils.CursorWrapper object at 0x7f9b3bce89e0>
sql = 'INSERT INTO "pulls" ("repoid", "pullid", "issueid", "state", "title", "base", "head", "user_provided_base_sha", "comp..._storage_path") VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) RETURNING "pulls"."id"'
params = (35, 1, 127, 'open', 'test-pull-request', 'd159549ac15dbe12d463e5fcc702afe6773ed8cf', ...)
ignored_wrapper_args = (False, {'connection': <DatabaseWrapper vendor='postgresql' alias='default'>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f9b3bce89e0>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "pulls_repoid_pullid"
E               DETAIL:  Key (repoid, pullid)=(35, 1) already exists.

.../local/lib/python3.12.../db/backends/utils.py:89: UniqueViolation

The above exception was the direct cause of the following exception:

self = <graphql_api.tests.test_pull.TestPullRequestList testMethod=test_compare_bundle_analysis_missing_reports>

    def test_compare_bundle_analysis_missing_reports(self):
        head = CommitFactory(
            repository=self.repository,
            author=self.owner,
            commitid="5672734ij1n234918231290j12nasdfioasud0f9",
            totals={"c": "78.38", "diff": [0, 0, 0, 0, 0, "14"]},
        )
        compared_to = CommitFactory(
            repository=self.repository,
            author=self.owner,
            commitid="9asd78fa7as8d8fa97s8d7fgagsd8fa9asd8f77s",
        )
    
>       my_pull = PullFactory(
            repository=self.repository,
            title="test-pull-request",
            author=self.owner,
            head=head.commitid,
            compared_to=compared_to.commitid,
            behind_by=23,
            behind_by_commit="1089nf898as-jdf09hahs09fgh",
        )

graphql_api/tests/test_pull.py:475: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.../local/lib/python3.12............/site-packages/factory/base.py:40: in __call__
    return cls.create(**kwargs)
.../local/lib/python3.12............/site-packages/factory/base.py:528: in create
    return cls._generate(enums.CREATE_STRATEGY, kwargs)
.../local/lib/python3.12....../site-packages/factory/django.py:117: in _generate
    return super()._generate(strategy, params)
.../local/lib/python3.12............/site-packages/factory/base.py:465: in _generate
    return step.build()
.../local/lib/python3.12.../site-packages/factory/builder.py:262: in build
    instance = self.factory_meta.instantiate(
.../local/lib/python3.12............/site-packages/factory/base.py:317: in instantiate
    return self.factory._create(model, *args, **kwargs)
.../local/lib/python3.12....../site-packages/factory/django.py:166: in _create
    return manager.create(*args, **kwargs)
.../local/lib/python3.12.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.12.../db/models/query.py:658: in create
    obj.save(force_insert=True, using=self.db)
.../local/lib/python3.12.../django_apps/core/models.py:446: in save
    super().save(*args, **kwargs)
.../local/lib/python3.12.../db/models/base.py:814: in save
    self.save_base(
.../local/lib/python3.12.../db/models/base.py:877: in save_base
    updated = self._save_table(
.../local/lib/python3.12.../db/models/base.py:1020: in _save_table
    results = self._do_insert(
.../local/lib/python3.12.../site-packages/django_prometheus/models.py:43: in _do_insert
    return super()._do_insert(*args, **kwargs)
.../local/lib/python3.12.../db/models/base.py:1061: in _do_insert
    return manager._insert(
.../local/lib/python3.12.../db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
.../local/lib/python3.12.../db/models/query.py:1805: in _insert
    return query.get_compiler(using=using).execute_sql(returning_fields)
.../local/lib/python3.12.../models/sql/compiler.py:1822: in execute_sql
    cursor.execute(sql, params)
.../local/lib/python3.12.../site-packages/sentry_sdk/utils.py:1730: in runner
    return sentry_patched_function(*args, **kwargs)
.../local/lib/python3.12.../integrations/django/__init__.py:651: in execute
    result = real_execute(self, sql, params)
.../local/lib/python3.12.../db/backends/utils.py:67: in execute
    return self._execute_with_wrappers(
.../local/lib/python3.12.../db/backends/utils.py:80: in _execute_with_wrappers
    return executor(sql, params, many, context)
.../local/lib/python3.12.../db/backends/utils.py:84: in _execute
    with self.db.wrap_database_errors:
.../local/lib/python3.12.../django/db/utils.py:91: in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <django.db.backends.utils.CursorWrapper object at 0x7f9b3bce89e0>
sql = 'INSERT INTO "pulls" ("repoid", "pullid", "issueid", "state", "title", "base", "head", "user_provided_base_sha", "comp..._storage_path") VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s) RETURNING "pulls"."id"'
params = (35, 1, 127, 'open', 'test-pull-request', 'd159549ac15dbe12d463e5fcc702afe6773ed8cf', ...)
ignored_wrapper_args = (False, {'connection': <DatabaseWrapper vendor='postgresql' alias='default'>, 'cursor': <django.db.backends.utils.CursorWrapper object at 0x7f9b3bce89e0>})

    def _execute(self, sql, params, *ignored_wrapper_args):
        self.db.validate_no_broken_transaction()
        with self.db.wrap_database_errors:
            if params is None:
                # params default might be backend specific.
                return self.cursor.execute(sql)
            else:
>               return self.cursor.execute(sql, params)
E               django.db.utils.IntegrityError: duplicate key value violates unique constraint "pulls_repoid_pullid"
E               DETAIL:  Key (repoid, pullid)=(35, 1) already exists.

.../local/lib/python3.12.../db/backends/utils.py:89: IntegrityError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

ajay-sentry · 2025-02-13T21:17:50Z

Makefile


 test_env.run_integration:
-	#docker-compose exec api make test.integration
+	# @if [ -n "$(GROUP)" ]; then \


comment this out if we ever have integration tests and it should just work

Do you know why is this command commented to begin with?

JerrySentry

THIS IS HUGE

ajay-sentry · 2025-02-13T21:44:09Z

.github/workflows/ci.yml

@@ -49,7 +49,7 @@ jobs:
  test:
    name: Test
    needs: [build]
-    uses: codecov/gha-workflows/.github/workflows/run-tests.yml@v1.2.27
+    uses: codecov/gha-workflows/.github/workflows/run-tests-split.yml@285163a75899bad2018fe960ac9dba7530e009fb


TODO: update this to the new release hash before merging

suejung-sentry

This is amazing - thank you for doing it for us!

suejung-sentry · 2025-02-13T22:47:25Z

graphql_api/tests/test_pull.py

-        assert pull == {
-            "bundleAnalysisCompareWithBase": {"__typename": "MissingBaseReport"}
-        }
+    # def test_compare_bundle_analysis_missing_reports(self):


maybe leave a comment why this is commented?

Yeah good point, we did create a ticket here so it doesn't get lost: codecov/engineering-team#3358, I can link that to the test directly too

suejung-sentry · 2025-02-13T22:48:34Z

Makefile


 test_env.run_integration:
-	#docker-compose exec api make test.integration
+	# @if [ -n "$(GROUP)" ]; then \


Do you know why is this command commented to begin with?

suejung-sentry · 2025-02-13T22:49:41Z

.test_durations

Is this file how long it took to run our suite? If it gets recommited with every PR, maybe we should just gitignore it?

Re: commented out command, it's bc we don't have integration tests in API (yet) 😅

Re: .test_durations, yes it is how long each individual test ran and it only gets "created" or "updated" when we pass in the --test-durations flag or something so it shouldn't be created unless we explicitly want to

its actually the --store-durations flag, I misspoke

…ns file

stuff required for pytest-split

c5c1f06

ajay-sentry added 8 commits February 12, 2025 15:48

try out the new workflow

3ef611a

update makefile adding split

66fba13

use group if exists or fallback

0dcc2e4

also update splits to be dynamic

8e42155

Merge branch 'main' into Ajay/pytest-split

6bb226f

test it

2c6d55f

plumb

326353d

fix thsi

a5ff4fe

comment out this test to run everything

9afdf21

ajay-sentry requested a review from a team as a code owner February 13, 2025 19:36

comment out integration tests

7dc524b

ajay-sentry commented Feb 13, 2025

View reviewed changes

ajay-sentry mentioned this pull request Feb 13, 2025

[Bundle Analysis] Fix test_compare_bundle_analysis_missing_reports test in test_pull.py codecov/engineering-team#3358

Open

JerrySentry approved these changes Feb 13, 2025

View reviewed changes

ajay-sentry closed this Feb 13, 2025

ajay-sentry reopened this Feb 13, 2025

ajay-sentry commented Feb 13, 2025

View reviewed changes

suejung-sentry approved these changes Feb 13, 2025

View reviewed changes

ajay-sentry added 4 commits February 14, 2025 09:20

Merge branch 'main' into Ajay/pytest-split

1e5e9e7

update xmls

648d7d0

skip test and add comment instead of comment out, update test duratio…

c67b728

…ns file

use matts version

3343bbe

ajay-sentry closed this Feb 14, 2025

ajay-sentry reopened this Feb 14, 2025

ajay-sentry closed this Feb 14, 2025

ajay-sentry reopened this Feb 14, 2025

ajay-sentry closed this Feb 14, 2025

ajay-sentry reopened this Feb 14, 2025

ajay-sentry closed this Feb 14, 2025

ajay-sentry reopened this Feb 14, 2025

matt-codecov closed this Feb 14, 2025

matt-codecov reopened this Feb 14, 2025

matt-codecov closed this Feb 14, 2025

matt-codecov reopened this Feb 14, 2025

matt-codecov closed this Feb 14, 2025

matt-codecov reopened this Feb 14, 2025

use latest split release

6e028a8

ajay-sentry enabled auto-merge February 18, 2025 21:41

ajay-sentry disabled auto-merge February 18, 2025 21:42

ajay-sentry enabled auto-merge February 18, 2025 21:42

ajay-sentry added this pull request to the merge queue Feb 18, 2025

Merged via the queue into main with commit de5e284 Feb 18, 2025
22 of 23 checks passed

ajay-sentry deleted the Ajay/pytest-split branch February 18, 2025 21:56

Swatinem mentioned this pull request Mar 17, 2025

Revert split tests #1213

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev: Pytest-split test #1144

dev: Pytest-split test #1144

ajay-sentry commented Feb 7, 2025 •

edited

Loading

codecov bot commented Feb 7, 2025 •

edited

Loading

codecov-notifications bot commented Feb 7, 2025 •

edited

Loading

codecov-qa bot commented Feb 7, 2025 •

edited

Loading

github-actions bot commented Feb 7, 2025

codecov-public-qa bot commented Feb 13, 2025 •

edited

Loading

ajay-sentry Feb 13, 2025

suejung-sentry Feb 13, 2025

JerrySentry left a comment

ajay-sentry Feb 13, 2025

suejung-sentry left a comment

suejung-sentry Feb 13, 2025

ajay-sentry Feb 13, 2025

ajay-sentry Feb 14, 2025

suejung-sentry Feb 13, 2025

suejung-sentry Feb 13, 2025

ajay-sentry Feb 13, 2025

ajay-sentry Feb 14, 2025

dev: Pytest-split test #1144

dev: Pytest-split test #1144

Conversation

ajay-sentry commented Feb 7, 2025 • edited Loading

Purpose/Motivation

UPDATE:

Legal Boilerplate

codecov bot commented Feb 7, 2025 • edited Loading

Codecov Report

codecov-notifications bot commented Feb 7, 2025 • edited Loading

Codecov Report

codecov-qa bot commented Feb 7, 2025 • edited Loading

Codecov Report

github-actions bot commented Feb 7, 2025

codecov-public-qa bot commented Feb 13, 2025 • edited Loading

❌ 1 Tests Failed:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JerrySentry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suejung-sentry left a 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

ajay-sentry commented Feb 7, 2025 •

edited

Loading

codecov bot commented Feb 7, 2025 •

edited

Loading

codecov-notifications bot commented Feb 7, 2025 •

edited

Loading

codecov-qa bot commented Feb 7, 2025 •

edited

Loading

codecov-public-qa bot commented Feb 13, 2025 •

edited

Loading