Remove usage of `ReportDetails`/`files_array` #911

Swatinem · 2024-10-22T13:10:22Z

This removes all usage of ReportDetails. The base definitions still exist.

codecov · 2024-10-22T13:21:45Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.22%. Comparing base (cdebb02) to head (90e7eb8).
Report is 9 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #911      +/-   ##
==========================================
- Coverage   96.23%   96.22%   -0.01%     
==========================================
  Files         823      823              
  Lines       18972    18963       -9     
==========================================
- Hits        18257    18248       -9     
  Misses        715      715

Flag	Coverage Δ
unit	`92.46% <100.00%> (-0.01%)`	⬇️
unit-latest-uploader	`92.46% <100.00%> (-0.01%)`	⬇️

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 · 2024-10-22T13:21:49Z

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!

github-actions · 2024-10-22T13:25:21Z

This PR includes changes to shared. Please review them here: codecov/shared@f0e213c...78f9525

github-actions · 2024-10-23T09:17:21Z

This PR includes changes to shared. Please review them here: codecov/shared@f0e213c...78f9525

github-actions · 2024-10-28T10:55:54Z

This PR includes changes to shared. Please review them here: codecov/shared@4e27927...067b2e4

codecov-qa · 2024-10-28T11:15:39Z

❌ 4 Tests Failed:

Tests completed	Failed	Passed	Skipped
2631	4	2627	6

View the top 3 failed tests by shortest run time

api/public/v2/tests/test_test_results_view.py::TestResultsViewsetTests::test_result_with_valid_super_token

Stack Traces | 0.406s run time

self = <test_test_results_view.TestResultsViewsetTests testMethod=test_result_with_valid_super_token>;
repository_artifact_permissions_has_permission = <MagicMock name='has_permission' id='139854765395888'>;

    @override_settings(SUPER_API_TOKEN="testaxs3o76rdcdpfzexuccx3uatui2nw73r")
    @patch("api.shared.permissions.RepositoryArtifactPermissions.has_permission")
    def test_result_with_valid_super_token(
        self, repository_artifact_permissions_has_permission
    ):
        repository_artifact_permissions_has_permission.return_value = False
        res = self.client.get(
            reverse(
                "api-v2-tests-results-detail",
                kwargs={
                    "service": self.org.service,
                    "owner_username": self.org.username,
                    "repo_name": self.repo.name,
                    "pk": self.test_instances[0].pk,
                },
            ),
            HTTP_AUTHORIZATION="Bearer testaxs3o76rdcdpfzexuccx3uatui2nw73r",
        )
        assert res.status_code == 200
        assert res.json() == {
            "id": self.test_instances[0].id,
            "name": self.test_instances[0].test.name,
            "test_id": self.test_instances[0].test_id,
            "failure_message": self.test_instances[0].failure_message,
            "duration_seconds": self.test_instances[0].duration_seconds,
            "commitid": self.test_instances[0].commitid,
            "outcome": self.test_instances[0].outcome,
            "branch": self.test_instances[0].branch,
            "repoid": self.test_instances[0].repoid,
            "failure_rate": self.test_instances[0].test.failure_rate,
            "commits_where_fail": self.test_instances[0].test.commits_where_fail,
&gt;       }
E       AttributeError: 'Test' object has no attribute 'failure_rate'

.../v2/tests/test_test_results_view.py:245: AttributeError

api/public/v2/tests/test_test_results_view.py::TestResultsViewsetTests::test_list_filters

Stack Traces | 0.425s run time

self = &lt;test_test_results_view.TestResultsViewsetTests testMethod=test_list_filters&gt;

    def test_list_filters(self):
        url = reverse(
            "api-v2-tests-results-list",
            kwargs={
                "service": self.org.service,
                "owner_username": self.org.username,
                "repo_name": self.repo.name,
            },
        )
        res = self.client.get(f"{url}?commit_id={self.test_instances[0].commitid}")
        assert res.status_code == status.HTTP_200_OK
        assert res.json() == {
            "count": 1,
            "next": None,
            "previous": None,
            "results": [
                {
                    "id": self.test_instances[0].id,
                    "name": self.test_instances[0].test.name,
                    "test_id": self.test_instances[0].test_id,
                    "failure_message": self.test_instances[0].failure_message,
                    "duration_seconds": self.test_instances[0].duration_seconds,
                    "commitid": self.test_instances[0].commitid,
                    "outcome": self.test_instances[0].outcome,
                    "branch": self.test_instances[0].branch,
                    "repoid": self.test_instances[0].repoid,
                    "failure_rate": self.test_instances[0].test.failure_rate,
                    "commits_where_fail": self.test_instances[
                        0
                    ].test.commits_where_fail,
                },
            ],
            "total_pages": 1,
&gt;       }
E       AttributeError: 'Test' object has no attribute 'failure_rate'

.../v2/tests/test_test_results_view.py:113: AttributeError

api/public/v2/tests/test_test_results_view.py::TestResultsViewsetTests::test_retrieve

Stack Traces | 0.429s run time

self = &lt;test_test_results_view.TestResultsViewsetTests testMethod=test_retrieve&gt;
get_repo_permissions = &lt;MagicMock name='get_repo_permissions' id='139855347634048'&gt;

    @patch("api.shared.repo.repository_accessors.RepoAccessors.get_repo_permissions")
    def test_retrieve(self, get_repo_permissions):
        get_repo_permissions.return_value = (True, True)
        res = self.client.get(
            reverse(
                "api-v2-tests-results-detail",
                kwargs={
                    "service": self.org.service,
                    "owner_username": self.org.username,
                    "repo_name": self.repo.name,
                    "pk": self.test_instances[0].pk,
                },
            )
        )
        assert res.status_code == status.HTTP_200_OK
        assert res.json() == {
            "id": self.test_instances[0].id,
            "name": self.test_instances[0].test.name,
            "test_id": self.test_instances[0].test_id,
            "failure_message": self.test_instances[0].failure_message,
            "duration_seconds": self.test_instances[0].duration_seconds,
            "commitid": self.test_instances[0].commitid,
            "outcome": self.test_instances[0].outcome,
            "branch": self.test_instances[0].branch,
            "repoid": self.test_instances[0].repoid,
            "failure_rate": self.test_instances[0].test.failure_rate,
            "commits_where_fail": self.test_instances[0].test.commits_where_fail,
&gt;       }
E       AttributeError: 'Test' object has no attribute 'failure_rate'

.../v2/tests/test_test_results_view.py:142: AttributeError

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

codecov-public-qa · 2024-10-28T11:15:39Z

Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time.

❌ Failed Test Results:

Completed 2637 tests with 4 failed, 2627 passed and 6 skipped.

View the full list of failed tests

pytest

Class name: api.public.v2.tests.test_test_results_view.TestResultsViewsetTests
Test name: test_list

self = <test_test_results_view.TestResultsViewsetTests testMethod=test_list>

    def test_list(self):
        url = reverse(
            "api-v2-tests-results-list",
            kwargs={
                "service": self.org.service,
                "owner_username": self.org.username,
                "repo_name": self.repo.name,
            },
        )
        res = self.client.get(url)
        assert res.status_code == status.HTTP_200_OK
        assert res.json() == {
            "count": 2,
            "next": None,
            "previous": None,
            "results": [
                {
                    "id": self.test_instances[0].id,
                    "name": self.test_instances[0].test.name,
                    "test_id": self.test_instances[0].test_id,
                    "failure_message": self.test_instances[0].failure_message,
                    "duration_seconds": self.test_instances[0].duration_seconds,
                    "commitid": self.test_instances[0].commitid,
                    "outcome": self.test_instances[0].outcome,
                    "branch": self.test_instances[0].branch,
                    "repoid": self.test_instances[0].repoid,
                    "failure_rate": self.test_instances[0].test.failure_rate,
                    "commits_where_fail": self.test_instances[
                        0
                    ].test.commits_where_fail,
                },
                {
                    "id": self.test_instances[1].id,
                    "name": self.test_instances[1].test.name,
                    "test_id": self.test_instances[1].test_id,
                    "failure_message": self.test_instances[1].failure_message,
                    "duration_seconds": self.test_instances[1].duration_seconds,
                    "commitid": self.test_instances[1].commitid,
                    "outcome": self.test_instances[1].outcome,
                    "branch": self.test_instances[1].branch,
                    "repoid": self.test_instances[1].repoid,
                    "failure_rate": self.test_instances[1].test.failure_rate,
                    "commits_where_fail": self.test_instances[
                        1
                    ].test.commits_where_fail,
                },
            ],
            "total_pages": 1,
>       }
E       AttributeError: 'Test' object has no attribute 'failure_rate'

.../v2/tests/test_test_results_view.py:78: AttributeError

Class name: api.public.v2.tests.test_test_results_view.TestResultsViewsetTests
Test name: test_list_filters

self = <test_test_results_view.TestResultsViewsetTests testMethod=test_list_filters>

    def test_list_filters(self):
        url = reverse(
            "api-v2-tests-results-list",
            kwargs={
                "service": self.org.service,
                "owner_username": self.org.username,
                "repo_name": self.repo.name,
            },
        )
        res = self.client.get(f"{url}&commit_id={self.test_instances[0].commitid}")
        assert res.status_code == status.HTTP_200_OK
        assert res.json() == {
            "count": 1,
            "next": None,
            "previous": None,
            "results": [
                {
                    "id": self.test_instances[0].id,
                    "name": self.test_instances[0].test.name,
                    "test_id": self.test_instances[0].test_id,
                    "failure_message": self.test_instances[0].failure_message,
                    "duration_seconds": self.test_instances[0].duration_seconds,
                    "commitid": self.test_instances[0].commitid,
                    "outcome": self.test_instances[0].outcome,
                    "branch": self.test_instances[0].branch,
                    "repoid": self.test_instances[0].repoid,
                    "failure_rate": self.test_instances[0].test.failure_rate,
                    "commits_where_fail": self.test_instances[
                        0
                    ].test.commits_where_fail,
                },
            ],
            "total_pages": 1,
>       }
E       AttributeError: 'Test' object has no attribute 'failure_rate'

.../v2/tests/test_test_results_view.py:113: AttributeError

Class name: api.public.v2.tests.test_test_results_view.TestResultsViewsetTests
Test name: test_result_with_valid_super_token

self = <test_test_results_view.TestResultsViewsetTests testMethod=test_result_with_valid_super_token>
repository_artifact_permissions_has_permission = <MagicMock name='has_permission' id='139854765395888'>

    @override_settings(SUPER_API_TOKEN="testaxs3o76rdcdpfzexuccx3uatui2nw73r")
    @patch("api.shared.permissions.RepositoryArtifactPermissions.has_permission")
    def test_result_with_valid_super_token(
        self, repository_artifact_permissions_has_permission
    ):
        repository_artifact_permissions_has_permission.return_value = False
        res = self.client.get(
            reverse(
                "api-v2-tests-results-detail",
                kwargs={
                    "service": self.org.service,
                    "owner_username": self.org.username,
                    "repo_name": self.repo.name,
                    "pk": self.test_instances[0].pk,
                },
            ),
            HTTP_AUTHORIZATION="Bearer testaxs3o76rdcdpfzexuccx3uatui2nw73r",
        )
        assert res.status_code == 200
        assert res.json() == {
            "id": self.test_instances[0].id,
            "name": self.test_instances[0].test.name,
            "test_id": self.test_instances[0].test_id,
            "failure_message": self.test_instances[0].failure_message,
            "duration_seconds": self.test_instances[0].duration_seconds,
            "commitid": self.test_instances[0].commitid,
            "outcome": self.test_instances[0].outcome,
            "branch": self.test_instances[0].branch,
            "repoid": self.test_instances[0].repoid,
            "failure_rate": self.test_instances[0].test.failure_rate,
            "commits_where_fail": self.test_instances[0].test.commits_where_fail,
>       }
E       AttributeError: 'Test' object has no attribute 'failure_rate'

.../v2/tests/test_test_results_view.py:245: AttributeError

Class name: api.public.v2.tests.test_test_results_view.TestResultsViewsetTests
Test name: test_retrieve

self = <test_test_results_view.TestResultsViewsetTests testMethod=test_retrieve>
get_repo_permissions = <MagicMock name='get_repo_permissions' id='139855347634048'>

    @patch("api.shared.repo.repository_accessors.RepoAccessors.get_repo_permissions")
    def test_retrieve(self, get_repo_permissions):
        get_repo_permissions.return_value = (True, True)
        res = self.client.get(
            reverse(
                "api-v2-tests-results-detail",
                kwargs={
                    "service": self.org.service,
                    "owner_username": self.org.username,
                    "repo_name": self.repo.name,
                    "pk": self.test_instances[0].pk,
                },
            )
        )
        assert res.status_code == status.HTTP_200_OK
        assert res.json() == {
            "id": self.test_instances[0].id,
            "name": self.test_instances[0].test.name,
            "test_id": self.test_instances[0].test_id,
            "failure_message": self.test_instances[0].failure_message,
            "duration_seconds": self.test_instances[0].duration_seconds,
            "commitid": self.test_instances[0].commitid,
            "outcome": self.test_instances[0].outcome,
            "branch": self.test_instances[0].branch,
            "repoid": self.test_instances[0].repoid,
            "failure_rate": self.test_instances[0].test.failure_rate,
            "commits_where_fail": self.test_instances[0].test.commits_where_fail,
>       }
E       AttributeError: 'Test' object has no attribute 'failure_rate'

.../v2/tests/test_test_results_view.py:142: AttributeError

This removes all usage of `ReportDetails`. The base definitions still exist.

suejung-sentry

I double checked and didn't see any impact of removing the ReportDetails, files_array usages as proposed here on the GraphQL apis and public REST apis.
Honestly, I'm puzzled why this was here in the first place? Did some digging in the history but didn't turn up anything obvious (my best guess is at some point we moved over from reading the results of the coverage reports in reports_reportdetails.files_array and moved to a path at GCS at commits.report_storage_path)?

All the places our frontend app fetches any files data, it appears to download from the GCS path at commits.report_storage_path, so that's good, unaffected by this change.

I also smoke-tested clicked around the deployment at https://stage-app.codecov.dev/gh/codecov and didn't see anything out of place. So I think we should be good to go with this!

(disclaimer: I only lightly educated myself on the needs_recompute logic that was edited in this PR but seems generally reasonable!)

Swatinem · 2024-10-31T10:21:53Z

Yes that was it, the ReportDetails.files_array was intended as a feature flagged replacement for the Commit.report_json (which is stored in GCS).

The feature flag never made it, and I wanted to be extra careful rolling back all the changes.

Swatinem self-assigned this Oct 22, 2024

Swatinem mentioned this pull request Oct 22, 2024

Consider deprecating/removing report_builder feature codecov/engineering-team#2554

Open

Swatinem force-pushed the swatinem/rm-reportdetails branch from 08efd1d to a22f1d5 Compare October 23, 2024 09:17

Swatinem force-pushed the swatinem/rm-reportdetails branch from a22f1d5 to b6f7e58 Compare October 28, 2024 10:55

Swatinem marked this pull request as ready for review October 28, 2024 10:55

Swatinem requested a review from a team as a code owner October 28, 2024 10:55

Swatinem requested a review from a team October 28, 2024 10:57

Remove usage of ReportDetails/files_array

90e7eb8

This removes all usage of `ReportDetails`. The base definitions still exist.

Swatinem force-pushed the swatinem/rm-reportdetails branch from b6f7e58 to 90e7eb8 Compare October 29, 2024 08:32

nicholas-codecov approved these changes Oct 30, 2024

View reviewed changes

giovanni-guidini approved these changes Oct 30, 2024

View reviewed changes

trent-codecov approved these changes Oct 30, 2024

View reviewed changes

Swatinem temporarily deployed to staging October 30, 2024 13:39 — with GitHub Actions Inactive

suejung-sentry approved these changes Oct 31, 2024

View reviewed changes

Swatinem added this pull request to the merge queue Oct 31, 2024

Merged via the queue into main with commit 4db2c66 Oct 31, 2024
29 of 32 checks passed

Swatinem deleted the swatinem/rm-reportdetails branch October 31, 2024 10:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove usage of `ReportDetails`/`files_array` #911

Remove usage of `ReportDetails`/`files_array` #911

Swatinem commented Oct 22, 2024

codecov bot commented Oct 22, 2024 •

edited

Loading

codecov-notifications bot commented Oct 22, 2024 •

edited

Loading

github-actions bot commented Oct 22, 2024

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 28, 2024

codecov-qa bot commented Oct 28, 2024 •

edited by ajay-sentry

Loading

codecov-public-qa bot commented Oct 28, 2024

pytest

suejung-sentry left a comment

Swatinem commented Oct 31, 2024

Remove usage of ReportDetails/files_array #911

Remove usage of ReportDetails/files_array #911

Conversation

Swatinem commented Oct 22, 2024

codecov bot commented Oct 22, 2024 • edited Loading

Codecov Report

codecov-notifications bot commented Oct 22, 2024 • edited Loading

Codecov Report

github-actions bot commented Oct 22, 2024

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 28, 2024

codecov-qa bot commented Oct 28, 2024 • edited by ajay-sentry Loading

❌ 4 Tests Failed:

codecov-public-qa bot commented Oct 28, 2024

❌ Failed Test Results:

pytest

suejung-sentry left a comment

Choose a reason for hiding this comment

Swatinem commented Oct 31, 2024

Remove usage of `ReportDetails`/`files_array` #911

Remove usage of `ReportDetails`/`files_array` #911

codecov bot commented Oct 22, 2024 •

edited

Loading

codecov-notifications bot commented Oct 22, 2024 •

edited

Loading

codecov-qa bot commented Oct 28, 2024 •

edited by ajay-sentry

Loading