Cleanup and optimize `TestResultsFinisher` #924

Swatinem · 2024-12-02T15:21:22Z

This cleans up the code a bit, and removes usage of statsd metrics.

Some performance improvements here include avoiding querying the related Upload of failed tests multiple times, as well as fetching redundant pull request information in the Notifier.

Another optimization was related to querying per-outcome aggregations first, before querying for failures specifically. This avoids a N+1 query fetching all the related Tests.

codecov-notifications · 2024-12-02T15:27:54Z

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 · 2024-12-02T15:27:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.99%. Comparing base (50e7312) to head (9713bb9).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #924   +/-   ##
=======================================
  Coverage   97.98%   97.99%           
=======================================
  Files         445      445           
  Lines       35771    35754   -17     
=======================================
- Hits        35051    35036   -15     
+ Misses        720      718    -2

Flag	Coverage Δ
integration	`42.11% <100.00%> (-0.04%)`	⬇️
unit	`90.62% <29.78%> (+0.03%)`	⬆️

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.

github-actions · 2024-12-02T15:28:15Z

✅ All tests successful. No failed tests were found.

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

joseph-sentry

just one fix needed i think

tasks/test_results_finisher.py

codecov-qa · 2024-12-05T09:55:18Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
1761	1	1760	1

View the top 1 failed tests by shortest run time

tasks/tests/unit/test_test_results_finisher.py::TestUploadTestFinisherTask::test_upload_finisher_task_call_no_success

Stack Traces | 0.147s run time

self = <worker.tasks.tests.unit.test_test_results_finisher.TestUploadTestFinisherTask object at 0x7fb1969cd160>
mocker = <pytest_mock.plugin.MockFixture object at 0x7fb194688f80>
mock_configuration = <shared.config.ConfigHelper object at 0x7fb19484a120>
dbsession = <sqlalchemy.orm.session.Session object at 0x7fb1941e6e40>
codecov_vcr = <vcr.cassette.Cassette object at 0x7fb1948481d0>
mock_storage = <shared.storage.memory.MemoryStorageService object at 0x7fb1941ee930>
mock_redis = <MagicMock name='_get_redis_instance_from_url()' id='140400678247536'>
celery_app = <Celery celery.tests at 0x7fb1948d8470>
test_results_mock_app = <MagicMock name='app' id='140400682175760'>
mock_repo_provider_comments = <MagicMock id='140400677656464'>
test_results_setup_no_instances = (78, Commit<cd76b0821854a780b60012aed85af0a8263004ad@repo<78>>, Pull<12@repo<78>>, None)

    @pytest.mark.integration
    def test_upload_finisher_task_call_no_success(
        self,
        mocker,
        mock_configuration,
        dbsession,
        codecov_vcr,
        mock_storage,
        mock_redis,
        celery_app,
        test_results_mock_app,
        mock_repo_provider_comments,
        test_results_setup_no_instances,
    ):
        mock_feature = mocker.patch("services.test_results.FLAKY_TEST_DETECTION")
        mock_feature.check_value.return_value = False
    
        repoid, commit, pull, _ = test_results_setup_no_instances
    
>       result = TestResultsFinisherTask().run_impl(
            dbsession,
            [
                [{"successful": False}],
            ],
            repoid=repoid,
            commitid=commit.commitid,
            commit_yaml={"codecov": {"max_report_age": False}},
        )

.../tests/unit/test_test_results_finisher.py:537: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tasks/test_results_finisher.py:82: in run_impl
    finisher_result = self.process_impl_within_lock(
tasks/test_results_finisher.py:152: in process_impl_within_lock
    pull = async_to_sync(fetch_and_update_pull_request_information_from_commit)(
.../local/lib/python3.12....../site-packages/asgiref/sync.py:277: in __call__
    return call_result.result()
.../local/lib/python3.12....../concurrent/futures/_base.py:449: in result
    return self.__get_result()
.../local/lib/python3.12....../concurrent/futures/_base.py:401: in __get_result
    raise self._exception
.../local/lib/python3.12....../site-packages/asgiref/sync.py:353: in main_wrap
    result = await self.awaitable(*args, **kwargs)
.../local/lib/python3.12.../site-packages/sentry_sdk/tracing_utils.py:645: in func_with_tracing
    return await func(*args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

repository_service = <MagicMock id='140400677656464'>
commit = Commit<cd76b0821854a780b60012aed85af0a8263004ad@repo<78>>
current_yaml = <shared.yaml.user_yaml.UserYaml object at 0x7fb1941c8830>

    @sentry_sdk.trace
    async def fetch_and_update_pull_request_information_from_commit(
        repository_service: TorngitBaseAdapter, commit, current_yaml
    ) -> Optional[EnrichedPull]:
        db_session = commit.get_db_session()
        pullid = commit.pullid
        if not commit.pullid:
            try:
>               pullid = await repository_service.find_pull_request(
                    commit=commit.commitid, branch=commit.branch
                )
E               TypeError: object MagicMock can't be used in 'await' expression

services/repository.py:423: TypeError

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

codecov-public-qa · 2024-12-05T09:55:25Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
1761	1	1760	1

View the top 1 failed tests by shortest run time

tasks/tests/unit/test_test_results_finisher.py::TestUploadTestFinisherTask::test_upload_finisher_task_call_no_success

Stack Traces | 0.147s run time

self = &lt;worker.tasks.tests.unit.test_test_results_finisher.TestUploadTestFinisherTask object at 0x7fb1969cd160&gt;
mocker = &lt;pytest_mock.plugin.MockFixture object at 0x7fb194688f80&gt;
mock_configuration = &lt;shared.config.ConfigHelper object at 0x7fb19484a120&gt;
dbsession = &lt;sqlalchemy.orm.session.Session object at 0x7fb1941e6e40&gt;
codecov_vcr = &lt;vcr.cassette.Cassette object at 0x7fb1948481d0&gt;
mock_storage = &lt;shared.storage.memory.MemoryStorageService object at 0x7fb1941ee930&gt;
mock_redis = &lt;MagicMock name='_get_redis_instance_from_url()' id='140400678247536'&gt;
celery_app = &lt;Celery celery.tests at 0x7fb1948d8470&gt;
test_results_mock_app = &lt;MagicMock name='app' id='140400682175760'&gt;
mock_repo_provider_comments = &lt;MagicMock id='140400677656464'&gt;
test_results_setup_no_instances = (78, Commit&lt;cd76b0821854a780b60012aed85af0a8263004ad@repo&lt;78&gt;&gt;, Pull&lt;12@repo&lt;78&gt;&gt;, None)

    @pytest.mark.integration
    def test_upload_finisher_task_call_no_success(
        self,
        mocker,
        mock_configuration,
        dbsession,
        codecov_vcr,
        mock_storage,
        mock_redis,
        celery_app,
        test_results_mock_app,
        mock_repo_provider_comments,
        test_results_setup_no_instances,
    ):
        mock_feature = mocker.patch("services.test_results.FLAKY_TEST_DETECTION")
        mock_feature.check_value.return_value = False
    
        repoid, commit, pull, _ = test_results_setup_no_instances
    
&gt;       result = TestResultsFinisherTask().run_impl(
            dbsession,
            [
                [{"successful": False}],
            ],
            repoid=repoid,
            commitid=commit.commitid,
            commit_yaml={"codecov": {"max_report_age": False}},
        )

.../tests/unit/test_test_results_finisher.py:537: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tasks/test_results_finisher.py:82: in run_impl
    finisher_result = self.process_impl_within_lock(
tasks/test_results_finisher.py:152: in process_impl_within_lock
    pull = async_to_sync(fetch_and_update_pull_request_information_from_commit)(
.../local/lib/python3.12....../site-packages/asgiref/sync.py:277: in __call__
    return call_result.result()
.../local/lib/python3.12....../concurrent/futures/_base.py:449: in result
    return self.__get_result()
.../local/lib/python3.12....../concurrent/futures/_base.py:401: in __get_result
    raise self._exception
.../local/lib/python3.12....../site-packages/asgiref/sync.py:353: in main_wrap
    result = await self.awaitable(*args, **kwargs)
.../local/lib/python3.12.../site-packages/sentry_sdk/tracing_utils.py:645: in func_with_tracing
    return await func(*args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

repository_service = &lt;MagicMock id='140400677656464'&gt;
commit = Commit&lt;cd76b0821854a780b60012aed85af0a8263004ad@repo&lt;78&gt;&gt;
current_yaml = &lt;shared.yaml.user_yaml.UserYaml object at 0x7fb1941c8830&gt;

    @sentry_sdk.trace
    async def fetch_and_update_pull_request_information_from_commit(
        repository_service: TorngitBaseAdapter, commit, current_yaml
    ) -&gt; Optional[EnrichedPull]:
        db_session = commit.get_db_session()
        pullid = commit.pullid
        if not commit.pullid:
            try:
&gt;               pullid = await repository_service.find_pull_request(
                    commit=commit.commitid, branch=commit.branch
                )
E               TypeError: object MagicMock can't be used in 'await' expression

services/repository.py:423: TypeError

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

This cleans up the code a bit, and removes usage of statsd metrics. Some performance improvements here include avoiding querying the related `Upload` of failed tests multiple times, as well as fetching redundant pull request information.

sentry-io · 2024-12-06T07:45:50Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ IntegrityError: (psycopg2.errors.UniqueViolation) duplicate key value violates unique constraint "pulls_repoid_pu... app.tasks.test_results.TestResultsFinisherTask View Issue

_{Did you find this useful? React with a 👍 or 👎}

Swatinem requested a review from joseph-sentry December 2, 2024 15:21

Swatinem self-assigned this Dec 2, 2024

joseph-sentry requested changes Dec 4, 2024

View reviewed changes

tasks/test_results_finisher.py Outdated Show resolved Hide resolved

Swatinem force-pushed the swatinem/cleanup-ta-finisher branch from 61106ea to f3a3531 Compare December 5, 2024 09:49

Swatinem requested a review from joseph-sentry December 5, 2024 09:49

Swatinem force-pushed the swatinem/cleanup-ta-finisher branch from f3a3531 to 7e00b14 Compare December 5, 2024 12:39

joseph-sentry approved these changes Dec 5, 2024

View reviewed changes

Cleanup and optimize TestResultsFinisher

9713bb9

This cleans up the code a bit, and removes usage of statsd metrics. Some performance improvements here include avoiding querying the related `Upload` of failed tests multiple times, as well as fetching redundant pull request information.

Swatinem force-pushed the swatinem/cleanup-ta-finisher branch from 7e00b14 to 9713bb9 Compare December 5, 2024 16:03

Swatinem added this pull request to the merge queue Dec 5, 2024

Merged via the queue into main with commit 96bed1f Dec 5, 2024
26 of 27 checks passed

Swatinem deleted the swatinem/cleanup-ta-finisher branch December 5, 2024 16:19

Swatinem mentioned this pull request Dec 6, 2024

Resolve notifier pull on demand #948

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup and optimize `TestResultsFinisher` #924

Cleanup and optimize `TestResultsFinisher` #924

Swatinem commented Dec 2, 2024

codecov-notifications bot commented Dec 2, 2024 •

edited

Loading

codecov bot commented Dec 2, 2024 •

edited

Loading

github-actions bot commented Dec 2, 2024

joseph-sentry left a comment

codecov-qa bot commented Dec 5, 2024

codecov-public-qa bot commented Dec 5, 2024

sentry-io bot commented Dec 6, 2024

Cleanup and optimize TestResultsFinisher #924

Cleanup and optimize TestResultsFinisher #924

Conversation

Swatinem commented Dec 2, 2024

codecov-notifications bot commented Dec 2, 2024 • edited Loading

Codecov Report

codecov bot commented Dec 2, 2024 • edited Loading

Codecov Report

github-actions bot commented Dec 2, 2024

joseph-sentry left a comment

Choose a reason for hiding this comment

codecov-qa bot commented Dec 5, 2024

❌ 1 Tests Failed:

codecov-public-qa bot commented Dec 5, 2024

❌ 1 Tests Failed:

sentry-io bot commented Dec 6, 2024

Suspect Issues

Cleanup and optimize `TestResultsFinisher` #924

Cleanup and optimize `TestResultsFinisher` #924

codecov-notifications bot commented Dec 2, 2024 •

edited

Loading

codecov bot commented Dec 2, 2024 •

edited

Loading