bundle analysis: add a paginated option to retrieve assets #798

JerrySentry · 2024-09-05T14:55:09Z

Add a assetsPaginated field to BundleReport that allows pagination of asset retrieval. This is done primarily to speed up asset trend retrieval.

New GQL schema:

type BundleReport {
  ...
  assetsPaginated(
    ordering: AssetOrdering
    orderingDirection: OrderingDirection
    first: Int
    after: String
    last: Int
    before: String
  ): AssetConnection
}

type AssetConnection {
  edges: [AssetEdge]!
  totalCount: Int!
  pageInfo: PageInfo!
}

type AssetEdge {
  cursor: String!
  node: BundleAsset!
}

enum AssetOrdering {
  NAME
  SIZE
  TYPE
}

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-staging · 2024-09-05T15:07:38Z

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-public-qa · 2024-09-05T15:07:49Z

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

❌ Failed Test Results:

Completed 2277 tests with 5 failed, 2266 passed and 6 skipped.

View the full list of failed tests

pytest

Class name: graphql_api.tests.test_commit.TestCommit
Test name: test_bundle_analysis_asset_filtering

self = <MagicMock name='asset_reports' id='139770595656144'>, args = ()
kwargs = {'asset_types': None, 'chunk_entry': None, 'chunk_initial': None}
expected = call(asset_types=None, chunk_entry=None, chunk_initial=None)
actual = call(asset_types=None, chunk_entry=None, chunk_initial=None, ordering_desc=True)
_error_message = <function NonCallableMock.assert_called_with.<locals>._error_message at 0x7f1ed0911bc0>
cause = None

    def assert_called_with(self, /, *args, **kwargs):
        """assert that the last call was made with the specified arguments.
    
        Raises an AssertionError if the args and keyword args passed in are
        different to the last call to the mock."""
        if self.call_args is None:
            expected = self._format_mock_call_signature(args, kwargs)
            actual = 'not called.'
            error_message = ('expected call not found.\nExpected: %s\n  Actual: %s'
                    % (expected, actual))
            raise AssertionError(error_message)
    
        def _error_message():
            msg = self._format_mock_failure_message(args, kwargs)
            return msg
        expected = self._call_matcher(_Call((args, kwargs), two=True))
        actual = self._call_matcher(self.call_args)
        if actual != expected:
            cause = expected if isinstance(expected, Exception) else None
>           raise AssertionError(_error_message()) from cause
E           AssertionError: expected call not found.
E           Expected: asset_reports(asset_types=None, chunk_entry=None, chunk_initial=None)
E             Actual: asset_reports(asset_types=None, chunk_entry=None, chunk_initial=None, ordering_desc=True)

.../local/lib/python3.12/unittest/mock.py:947: AssertionError

During handling of the above exception, another exception occurred:

self = <graphql_api.tests.test_commit.TestCommit testMethod=test_bundle_analysis_asset_filtering>
get_storage_service = <MagicMock name='get_appropriate_storage_service' id='139770598356560'>
asset_reports_mock = <MagicMock name='asset_reports' id='139770595656144'>

    @patch("shared.bundle_analysis.BundleReport.asset_reports")
    @patch("graphql_api.dataloader.bundle_analysis.get_appropriate_storage_service")
    def test_bundle_analysis_asset_filtering(
        self, get_storage_service, asset_reports_mock
    ):
        storage = MemoryStorageService({})
    
        get_storage_service.return_value = storage
        asset_reports_mock.return_value = []
    
        head_commit_report = CommitReportFactory(
            commit=self.commit, report_type=CommitReport.ReportType.BUNDLE_ANALYSIS
        )
    
        with open(
            "..../tests/samples/bundle_with_assets_and_modules.sqlite", "rb"
        ) as f:
            storage_path = StoragePaths.bundle_report.path(
                repo_key=ArchiveService.get_archive_hash(self.repo),
                report_key=head_commit_report.external_id,
            )
            storage.write_file(get_bucket_name(), storage_path, f)
    
        query = """
            query FetchCommit($org: String!, $repo: String!, $commit: String!, $filters: BundleAnalysisReportFilters) {
                owner(username: $org) {
                    repository(name: $repo) {
                        ... on Repository {
                            commit(id: $commit) {
                                bundleAnalysisReport {
                                    __typename
                                    ... on BundleAnalysisReport {
                                        bundle(name: "b5", filters: $filters) {
                                            moduleCount
                                            assets {
                                                name
                                            }
                                        }
                                    }
                                }
                            }
                        }
                    }
                }
            }
        """
    
        variables = {
            "org": self.org.username,
            "repo": self.repo.name,
            "commit": self.commit.commitid,
            "filters": {},
        }
    
        configurations = [
            # No filters
            (
                {"loadTypes": None, "reportGroups": None},
                {"asset_types": None, "chunk_entry": None, "chunk_initial": None},
            ),
            ({}, {"asset_types": None, "chunk_entry": None, "chunk_initial": None}),
            # Just report groups
            (
                {"reportGroups": ["JAVASCRIPT", "FONT"]},
                {
                    "asset_types": ["JAVASCRIPT", "FONT"],
                    "chunk_entry": None,
                    "chunk_initial": None,
                },
            ),
            # Load types -> chunk_entry cancels out
            (
                {"loadTypes": ["ENTRY", "INITIAL"]},
                {"asset_types": None, "chunk_entry": None, "chunk_initial": True},
            ),
            # Load types -> chunk_entry = True
            (
                {"loadTypes": ["ENTRY"]},
                {"asset_types": None, "chunk_entry": True, "chunk_initial": None},
            ),
            # Load types -> chunk_lazy = False
            (
                {"loadTypes": ["LAZY"]},
                {"asset_types": None, "chunk_entry": False, "chunk_initial": False},
            ),
            # Load types -> chunk_initial cancels out
            (
                {"loadTypes": ["LAZY", "INITIAL"]},
                {"asset_types": None, "chunk_entry": False, "chunk_initial": None},
            ),
            # Load types -> chunk_initial = True
            (
                {"loadTypes": ["INITIAL"]},
                {"asset_types": None, "chunk_entry": False, "chunk_initial": True},
            ),
            # Load types -> chunk_initial = False
            (
                {"loadTypes": ["LAZY"]},
                {"asset_types": None, "chunk_entry": False, "chunk_initial": False},
            ),
        ]
    
        for config in configurations:
            input_d, output_d = config
            variables["filters"] = input_d
            data = self.gql_request(query, variables=variables)
            assert (
                data["owner"]["repository"]["commit"]["bundleAnalysisReport"]["bundle"]
                is not None
            )
>           asset_reports_mock.assert_called_with(**output_d)
E           AssertionError: expected call not found.
E           Expected: asset_reports(asset_types=None, chunk_entry=None, chunk_initial=None)
E             Actual: asset_reports(asset_types=None, chunk_entry=None, chunk_initial=None, ordering_desc=True)
E           
E           pytest introspection follows:
E           
E           Kwargs:
E           assert {'asset_types...g_desc': True} == {'asset_types...nitial': None}
E             
E             Omitting 3 identical items, use -vv to show
E             Left contains 1 more item:
E             {'ordering_desc': True}
E             Use -v to get more diff

graphql_api/tests/test_commit.py:1571: AssertionError

Class name: graphql_api.tests.test_commit.TestCommit
Test name: test_bundle_analysis_report

self = <graphql_api.tests.test_commit.TestCommit testMethod=test_bundle_analysis_report>
get_storage_service = <MagicMock name='get_appropriate_storage_service' id='139770921668720'>

    @patch("graphql_api.dataloader.bundle_analysis.get_appropriate_storage_service")
    def test_bundle_analysis_report(self, get_storage_service):
        storage = MemoryStorageService({})
        get_storage_service.return_value = storage
    
        head_commit_report = CommitReportFactory(
            commit=self.commit, report_type=CommitReport.ReportType.BUNDLE_ANALYSIS
        )
    
        with open("..../tests/samples/head_bundle_report.sqlite", "rb") as f:
            storage_path = StoragePaths.bundle_report.path(
                repo_key=ArchiveService.get_archive_hash(self.repo),
                report_key=head_commit_report.external_id,
            )
            storage.write_file(get_bucket_name(), storage_path, f)
    
        query = """
            query FetchCommit($org: String!, $repo: String!, $commit: String!) {
                owner(username: $org) {
                    repository(name: $repo) {
                        ... on Repository {
                            commit(id: $commit) {
                                bundleAnalysisReport {
                                    __typename
                                    ... on BundleAnalysisReport {
                                        bundles {
                                            name
                                            assets {
                                                normalizedName
                                            }
                                            asset(name: "not_exist") {
                                                normalizedName
                                            }
                                            bundleData {
                                                loadTime {
                                                    threeG
                                                    highSpeed
                                                }
                                                size {
                                                    gzip
                                                    uncompress
                                                }
                                            }
                                            isCached
                                        }
                                        bundleData {
                                            loadTime {
                                                threeG
                                                highSpeed
                                            }
                                            size {
                                                gzip
                                                uncompress
                                            }
                                        }
                                        bundle(name: "not_exist") {
                                            name
                                            isCached
                                        }
                                        isCached
                                    }
                                    ... on MissingHeadReport {
                                        message
                                    }
                                }
                            }
                        }
                    }
                }
            }
        """
    
        variables = {
            "org": self.org.username,
            "repo": self.repo.name,
            "commit": self.commit.commitid,
        }
        data = self.gql_request(query, variables=variables)
        commit = data["owner"]["repository"]["commit"]
    
>       assert commit["bundleAnalysisReport"] == {
            "__typename": "BundleAnalysisReport",
            "bundles": [
                {
                    "name": "b1",
                    "assets": [
                        {"normalizedName": "assets/react-*.svg"},
                        {"normalizedName": "assets/index-*.css"},
                        {"normalizedName": "assets/LazyComponent-*.js"},
                        {"normalizedName": "assets/index-*.js"},
                        {"normalizedName": "assets/index-*.js"},
                    ],
                    "asset": None,
                    "bundleData": {
                        "loadTime": {
                            "threeG": 0,
                            "highSpeed": 0,
                        },
                        "size": {
                            "gzip": 0,
                            "uncompress": 20,
                        },
                    },
                    "isCached": False,
                },
                {
                    "name": "b2",
                    "assets": [
                        {"normalizedName": "assets/react-*.svg"},
                        {"normalizedName": "assets/index-*.css"},
                        {"normalizedName": "assets/LazyComponent-*.js"},
                        {"normalizedName": "assets/index-*.js"},
                        {"normalizedName": "assets/index-*.js"},
                    ],
                    "asset": None,
                    "bundleData": {
                        "loadTime": {
                            "threeG": 2,
                            "highSpeed": 0,
                        },
                        "size": {
                            "gzip": 0,
                            "uncompress": 200,
                        },
                    },
                    "isCached": False,
                },
                {
                    "name": "b3",
                    "assets": [
                        {"normalizedName": "assets/react-*.svg"},
                        {"normalizedName": "assets/index-*.css"},
                        {"normalizedName": "assets/LazyComponent-*.js"},
                        {"normalizedName": "assets/index-*.js"},
                        {"normalizedName": "assets/index-*.js"},
                    ],
                    "asset": None,
                    "bundleData": {
                        "loadTime": {
                            "threeG": 16,
                            "highSpeed": 0,
                        },
                        "size": {
                            "gzip": 1,
                            "uncompress": 1500,
                        },
                    },
                    "isCached": False,
                },
                {
                    "name": "b5",
                    "assets": [
                        {"normalizedName": "assets/react-*.svg"},
                        {"normalizedName": "assets/index-*.css"},
                        {"normalizedName": "assets/LazyComponent-*.js"},
                        {"normalizedName": "assets/index-*.js"},
                        {"normalizedName": "assets/index-*.js"},
                    ],
                    "asset": None,
                    "bundleData": {
                        "loadTime": {
                            "threeG": 2133,
                            "highSpeed": 53,
                        },
                        "size": {
                            "gzip": 200,
                            "uncompress": 200000,
                        },
                    },
                    "isCached": False,
                },
            ],
            "bundleData": {
                "loadTime": {
                    "threeG": 2151,
                    "highSpeed": 53,
                },
                "size": {
                    "gzip": 201,
                    "uncompress": 201720,
                },
            },
            "bundle": None,
            "isCached": False,
        }
E       AssertionError: assert {'__typename'...e, ...}], ...} == {'__typename'...e, ...}], ...}
E         
E         Omitting 4 identical items, use -vv to show
E         Differing items:
E         {'bundles': [{'asset': None, 'assets': [{'normalizedName': 'assets/index-*.js'}, {'normalizedName': 'assets/index-*.js...{'loadTime': {'highSpeed': 53, 'threeG': 2133}, 'size': {'gzip': 200, 'uncompress': 200000}}, 'isCached': False, ...}]} != {'bundles': [{'asset': None, 'assets': [{'normalizedName': 'assets/react-*.svg'}, {'normalizedName': 'assets/index-*.c...{'loadTime': {'highSpeed': 53, 'threeG': 2133}, 'size': {'gzip': 200, 'uncompress': 200000}}, 'isCached': False, ...}]}
E         Use -v to get more diff

graphql_api/tests/test_commit.py:1216: AssertionError

Class name: graphql_api.tests.test_pull.TestPullRequestList
Test name: test_bundle_analysis_sqlite_file_deleted

self = <graphql_api.tests.test_pull.TestPullRequestList testMethod=test_bundle_analysis_sqlite_file_deleted>
get_storage_service = <MagicMock name='get_appropriate_storage_service' id='139770357905216'>

    @patch("graphql_api.dataloader.bundle_analysis.get_appropriate_storage_service")
    def test_bundle_analysis_sqlite_file_deleted(self, get_storage_service):
        os.system("rm -rf /tmp/bundle_analysis_*")
        storage = MemoryStorageService({})
        get_storage_service.return_value = storage
    
        parent_commit = CommitFactory(repository=self.repository)
        commit = CommitFactory(
            repository=self.repository,
            totals={"c": "12", "diff": [0, 0, 0, 0, 0, "14"]},
            parent_commit_id=parent_commit.commitid,
        )
    
        base_commit_report = CommitReportFactory(
            commit=parent_commit,
            report_type=CommitReport.ReportType.BUNDLE_ANALYSIS,
        )
        head_commit_report = CommitReportFactory(
            commit=commit, report_type=CommitReport.ReportType.BUNDLE_ANALYSIS
        )
    
        my_pull = PullFactory(
            repository=self.repository,
            title="test-pull-request",
            author=self.owner,
            head=head_commit_report.commit.commitid,
            compared_to=base_commit_report.commit.commitid,
            behind_by=23,
            behind_by_commit="1089nf898as-jdf09hahs09fgh",
        )
    
        with open("..../tests/samples/base_bundle_report.sqlite", "rb") as f:
            storage_path = StoragePaths.bundle_report.path(
                repo_key=ArchiveService.get_archive_hash(self.repository),
                report_key=base_commit_report.external_id,
            )
            storage.write_file(get_bucket_name(), storage_path, f)
    
        with open("..../tests/samples/head_bundle_report.sqlite", "rb") as f:
            storage_path = StoragePaths.bundle_report.path(
                repo_key=ArchiveService.get_archive_hash(self.repository),
                report_key=head_commit_report.external_id,
            )
            storage.write_file(get_bucket_name(), storage_path, f)
    
        query = """
            bundleAnalysisCompareWithBase {
                __typename
                ... on BundleAnalysisComparison {
                    bundleData {
                        size {
                            uncompress
                        }
                    }
                }
            }
        """
    
        pull = self.fetch_one_pull_request(my_pull.pullid, query)
    
>       assert pull == {
            "bundleAnalysisCompareWithBase": {
                "__typename": "BundleAnalysisComparison",
                "bundleData": {
                    "size": {
                        "uncompress": 201720,
                    }
                },
            }
        }
E       AssertionError: assert {'bundleAnaly...thBase': None} == {'bundleAnaly...s': 201720}}}}
E         
E         Differing items:
E         {'bundleAnalysisCompareWithBase': None} != {'bundleAnalysisCompareWithBase': {'__typename': 'BundleAnalysisComparison', 'bundleData': {'size': {'uncompress': 201720}}}}
E         Use -v to get more diff

graphql_api/tests/test_pull.py:555: AssertionError

Class name: graphql_api.tests.test_pull.TestPullRequestList
Test name: test_when_repository_has_null_head_has_parent_report

self = <graphql_api.tests.test_pull.TestPullRequestList testMethod=test_when_repository_has_null_head_has_parent_report>
get_storage_service = <MagicMock name='get_appropriate_storage_service' id='139770380553632'>

    @patch("graphql_api.dataloader.bundle_analysis.get_appropriate_storage_service")
    def test_when_repository_has_null_head_has_parent_report(self, get_storage_service):
        os.system("rm -rf /tmp/bundle_analysis_*")
        storage = MemoryStorageService({})
        get_storage_service.return_value = storage
    
        parent_commit = CommitFactory(repository=self.repository)
    
        base_commit_report = CommitReportFactory(
            commit=parent_commit,
            report_type=CommitReport.ReportType.BUNDLE_ANALYSIS,
        )
    
        my_pull = PullFactory(
            repository=self.repository,
            title="test-pull-request",
            author=self.owner,
            head=None,
            compared_to=base_commit_report.commit.commitid,
            behind_by=23,
            behind_by_commit="1089nf898as-jdf09hahs09fgh",
        )
    
        with open("..../tests/samples/base_bundle_report.sqlite", "rb") as f:
            storage_path = StoragePaths.bundle_report.path(
                repo_key=ArchiveService.get_archive_hash(self.repository),
                report_key=base_commit_report.external_id,
            )
            storage.write_file(get_bucket_name(), storage_path, f)
    
        query = """
            bundleAnalysisCompareWithBase {
                __typename
                ... on BundleAnalysisComparison {
                    bundleData {
                        size {
                            uncompress
                        }
                    }
                    bundleChange {
                        size {
                            uncompress
                        }
                    }
                }
            }
        """
    
        pull = self.fetch_one_pull_request(my_pull.pullid, query)
    
>       assert pull == {
            "bundleAnalysisCompareWithBase": {
                "__typename": "BundleAnalysisComparison",
                "bundleData": {
                    "size": {
                        "uncompress": 165165,
                    }
                },
                "bundleChange": {
                    "size": {
                        "uncompress": 0,
                    }
                },
            }
        }
E       AssertionError: assert {'bundleAnaly...thBase': None} == {'bundleAnaly...s': 165165}}}}
E         
E         Differing items:
E         {'bundleAnalysisCompareWithBase': None} != {'bundleAnalysisCompareWithBase': {'__typename': 'BundleAnalysisComparison', 'bundleChange': {'size': {'uncompress': 0}}, 'bundleData': {'size': {'uncompress': 165165}}}}
E         Use -v to get more diff

graphql_api/tests/test_pull.py:324: AssertionError

Class name: graphql_api.tests.test_repository.TestFetchRepository
Test name: test_fetch_is_github_rate_limited_but_errors

self = <MagicMock name='warning' id='139770933733056'>
args = ('Error when checking rate limit',)
kwargs = {'extra': {'has_owner': True, 'repo_id': 1826}}
msg = "Expected 'warning' to be called once. Called 2 times.\nCalls: [call('Too many event processors on scope! Clearing lis...processor at 0x7f1f046639c0>]),\n call('Error when checking rate limit', extra={'repo_id': 1826, 'has_owner': True})]."

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
>           raise AssertionError(msg)
E           AssertionError: Expected 'warning' to be called once. Called 2 times.
E           Calls: [call('Too many event processors on scope! Clearing list to free up some memory: %r', [<function _make_wsgi_request_event_processor.<locals>.wsgi_request_event_processor at 0x7f1f046639c0>]),
E            call('Error when checking rate limit', extra={'repo_id': 1826, 'has_owner': True})].

.../local/lib/python3.12/unittest/mock.py:958: AssertionError

During handling of the above exception, another exception occurred:

self = <graphql_api.tests.test_repository.TestFetchRepository testMethod=test_fetch_is_github_rate_limited_but_errors>
mock_log_warning = <MagicMock name='warning' id='139770933733056'>
mock_determine_rate_limit = <MagicMock name='determine_if_entity_is_rate_limited' id='139771234619552'>
mock_determine_redis_key = <MagicMock name='determine_entity_redis_key' id='139771064210720'>

    @patch("shared.rate_limits.determine_entity_redis_key")
    @patch("shared.rate_limits.determine_if_entity_is_rate_limited")
    @patch("logging.Logger.warning")
    @override_settings(IS_ENTERPRISE=True, GUEST_ACCESS=False)
    def test_fetch_is_github_rate_limited_but_errors(
        self,
        mock_log_warning,
        mock_determine_rate_limit,
        mock_determine_redis_key,
    ):
        repo = RepositoryFactory(
            author=self.owner,
            active=True,
            private=True,
            yaml={"component_management": {}},
        )
    
        mock_determine_redis_key.side_effect = Exception("some random error lol")
        mock_determine_rate_limit.return_value = True
    
        data = self.gql_request(
            query_repository
            % """
                isGithubRateLimited
            """,
            owner=self.owner,
            variables={"name": repo.name},
        )
    
        assert data["me"]["owner"]["repository"]["isGithubRateLimited"] is None
    
>       mock_log_warning.assert_called_once_with(
            "Error when checking rate limit",
            extra={
                "repo_id": repo.repoid,
                "has_owner": True,
            },
        )
E       AssertionError: Expected 'warning' to be called once. Called 2 times.
E       Calls: [call('Too many event processors on scope! Clearing list to free up some memory: %r', [<function _make_wsgi_request_event_processor.<locals>.wsgi_request_event_processor at 0x7f1f046639c0>]),
E        call('Error when checking rate limit', extra={'repo_id': 1826, 'has_owner': True})].

graphql_api/tests/test_repository.py:837: AssertionError

codecov-qa · 2024-09-05T15:10:45Z

❌ 5 Tests Failed:

Tests completed	Failed	Passed	Skipped
2271	5	2266	6

View the top 3 failed tests by shortest run time

graphql_api.tests.test_repository.TestFetchRepository test_fetch_is_github_rate_limited_but_errors

Stack Traces | 0.387s run time

self = &lt;MagicMock name='warning' id='139770933733056'&gt;
args = ('Error when checking rate limit',)
kwargs = {'extra': {'has_owner': True, 'repo_id': 1826}}
msg = "Expected 'warning' to be called once. Called 2 times.\nCalls: [call('Too many event processors on scope! Clearing lis...processor at 0x7f1f046639c0&gt;]),\n call('Error when checking rate limit', extra={'repo_id': 1826, 'has_owner': True})]."

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
&gt;           raise AssertionError(msg)
E           AssertionError: Expected 'warning' to be called once. Called 2 times.
E           Calls: [call('Too many event processors on scope! Clearing list to free up some memory: %r', [&lt;function _make_wsgi_request_event_processor.&lt;locals&gt;.wsgi_request_event_processor at 0x7f1f046639c0&gt;]),
E            call('Error when checking rate limit', extra={'repo_id': 1826, 'has_owner': True})].

.../local/lib/python3.12/unittest/mock.py:958: AssertionError

During handling of the above exception, another exception occurred:

self = &lt;graphql_api.tests.test_repository.TestFetchRepository testMethod=test_fetch_is_github_rate_limited_but_errors&gt;
mock_log_warning = &lt;MagicMock name='warning' id='139770933733056'&gt;
mock_determine_rate_limit = &lt;MagicMock name='determine_if_entity_is_rate_limited' id='139771234619552'&gt;
mock_determine_redis_key = &lt;MagicMock name='determine_entity_redis_key' id='139771064210720'&gt;

    @patch("shared.rate_limits.determine_entity_redis_key")
    @patch("shared.rate_limits.determine_if_entity_is_rate_limited")
    @patch("logging.Logger.warning")
    @override_settings(IS_ENTERPRISE=True, GUEST_ACCESS=False)
    def test_fetch_is_github_rate_limited_but_errors(
        self,
        mock_log_warning,
        mock_determine_rate_limit,
        mock_determine_redis_key,
    ):
        repo = RepositoryFactory(
            author=self.owner,
            active=True,
            private=True,
            yaml={"component_management": {}},
        )
    
        mock_determine_redis_key.side_effect = Exception("some random error lol")
        mock_determine_rate_limit.return_value = True
    
        data = self.gql_request(
            query_repository
            % """
                isGithubRateLimited
            """,
            owner=self.owner,
            variables={"name": repo.name},
        )
    
        assert data["me"]["owner"]["repository"]["isGithubRateLimited"] is None
    
&gt;       mock_log_warning.assert_called_once_with(
            "Error when checking rate limit",
            extra={
                "repo_id": repo.repoid,
                "has_owner": True,
            },
        )
E       AssertionError: Expected 'warning' to be called once. Called 2 times.
E       Calls: [call('Too many event processors on scope! Clearing list to free up some memory: %r', [&lt;function _make_wsgi_request_event_processor.&lt;locals&gt;.wsgi_request_event_processor at 0x7f1f046639c0&gt;]),
E        call('Error when checking rate limit', extra={'repo_id': 1826, 'has_owner': True})].

graphql_api/tests/test_repository.py:837: AssertionError

graphql_api.tests.test_commit.TestCommit test_bundle_analysis_asset_filtering

Stack Traces | 0.456s run time

self = &lt;MagicMock name='asset_reports' id='139770595656144'&gt;, args = ()
kwargs = {'asset_types': None, 'chunk_entry': None, 'chunk_initial': None}
expected = call(asset_types=None, chunk_entry=None, chunk_initial=None)
actual = call(asset_types=None, chunk_entry=None, chunk_initial=None, ordering_desc=True)
_error_message = &lt;function NonCallableMock.assert_called_with.&lt;locals&gt;._error_message at 0x7f1ed0911bc0&gt;
cause = None

    def assert_called_with(self, /, *args, **kwargs):
        """assert that the last call was made with the specified arguments.
    
        Raises an AssertionError if the args and keyword args passed in are
        different to the last call to the mock."""
        if self.call_args is None:
            expected = self._format_mock_call_signature(args, kwargs)
            actual = 'not called.'
            error_message = ('expected call not found.\nExpected: %s\n  Actual: %s'
                    % (expected, actual))
            raise AssertionError(error_message)
    
        def _error_message():
            msg = self._format_mock_failure_message(args, kwargs)
            return msg
        expected = self._call_matcher(_Call((args, kwargs), two=True))
        actual = self._call_matcher(self.call_args)
        if actual != expected:
            cause = expected if isinstance(expected, Exception) else None
&gt;           raise AssertionError(_error_message()) from cause
E           AssertionError: expected call not found.
E           Expected: asset_reports(asset_types=None, chunk_entry=None, chunk_initial=None)
E             Actual: asset_reports(asset_types=None, chunk_entry=None, chunk_initial=None, ordering_desc=True)

.../local/lib/python3.12/unittest/mock.py:947: AssertionError

During handling of the above exception, another exception occurred:

self = &lt;graphql_api.tests.test_commit.TestCommit testMethod=test_bundle_analysis_asset_filtering&gt;
get_storage_service = &lt;MagicMock name='get_appropriate_storage_service' id='139770598356560'&gt;
asset_reports_mock = &lt;MagicMock name='asset_reports' id='139770595656144'&gt;

    @patch("shared.bundle_analysis.BundleReport.asset_reports")
    @patch("graphql_api.dataloader.bundle_analysis.get_appropriate_storage_service")
    def test_bundle_analysis_asset_filtering(
        self, get_storage_service, asset_reports_mock
    ):
        storage = MemoryStorageService({})
    
        get_storage_service.return_value = storage
        asset_reports_mock.return_value = []
    
        head_commit_report = CommitReportFactory(
            commit=self.commit, report_type=CommitReport.ReportType.BUNDLE_ANALYSIS
        )
    
        with open(
            "..../tests/samples/bundle_with_assets_and_modules.sqlite", "rb"
        ) as f:
            storage_path = StoragePaths.bundle_report.path(
                repo_key=ArchiveService.get_archive_hash(self.repo),
                report_key=head_commit_report.external_id,
            )
            storage.write_file(get_bucket_name(), storage_path, f)
    
        query = """
            query FetchCommit($org: String!, $repo: String!, $commit: String!, $filters: BundleAnalysisReportFilters) {
                owner(username: $org) {
                    repository(name: $repo) {
                        ... on Repository {
                            commit(id: $commit) {
                                bundleAnalysisReport {
                                    __typename
                                    ... on BundleAnalysisReport {
                                        bundle(name: "b5", filters: $filters) {
                                            moduleCount
                                            assets {
                                                name
                                            }
                                        }
                                    }
                                }
                            }
                        }
                    }
                }
            }
        """
    
        variables = {
            "org": self.org.username,
            "repo": self.repo.name,
            "commit": self.commit.commitid,
            "filters": {},
        }
    
        configurations = [
            # No filters
            (
                {"loadTypes": None, "reportGroups": None},
                {"asset_types": None, "chunk_entry": None, "chunk_initial": None},
            ),
            ({}, {"asset_types": None, "chunk_entry": None, "chunk_initial": None}),
            # Just report groups
            (
                {"reportGroups": ["JAVASCRIPT", "FONT"]},
                {
                    "asset_types": ["JAVASCRIPT", "FONT"],
                    "chunk_entry": None,
                    "chunk_initial": None,
                },
            ),
            # Load types -&gt; chunk_entry cancels out
            (
                {"loadTypes": ["ENTRY", "INITIAL"]},
                {"asset_types": None, "chunk_entry": None, "chunk_initial": True},
            ),
            # Load types -&gt; chunk_entry = True
            (
                {"loadTypes": ["ENTRY"]},
                {"asset_types": None, "chunk_entry": True, "chunk_initial": None},
            ),
            # Load types -&gt; chunk_lazy = False
            (
                {"loadTypes": ["LAZY"]},
                {"asset_types": None, "chunk_entry": False, "chunk_initial": False},
            ),
            # Load types -&gt; chunk_initial cancels out
            (
                {"loadTypes": ["LAZY", "INITIAL"]},
                {"asset_types": None, "chunk_entry": False, "chunk_initial": None},
            ),
            # Load types -&gt; chunk_initial = True
            (
                {"loadTypes": ["INITIAL"]},
                {"asset_types": None, "chunk_entry": False, "chunk_initial": True},
            ),
            # Load types -&gt; chunk_initial = False
            (
                {"loadTypes": ["LAZY"]},
                {"asset_types": None, "chunk_entry": False, "chunk_initial": False},
            ),
        ]
    
        for config in configurations:
            input_d, output_d = config
            variables["filters"] = input_d
            data = self.gql_request(query, variables=variables)
            assert (
                data["owner"]["repository"]["commit"]["bundleAnalysisReport"]["bundle"]
                is not None
            )
&gt;           asset_reports_mock.assert_called_with(**output_d)
E           AssertionError: expected call not found.
E           Expected: asset_reports(asset_types=None, chunk_entry=None, chunk_initial=None)
E             Actual: asset_reports(asset_types=None, chunk_entry=None, chunk_initial=None, ordering_desc=True)
E           
E           pytest introspection follows:
E           
E           Kwargs:
E           assert {'asset_types...g_desc': True} == {'asset_types...nitial': None}
E             
E             Omitting 3 identical items, use -vv to show
E             Left contains 1 more item:
E             {'ordering_desc': True}
E             Use -v to get more diff

graphql_api/tests/test_commit.py:1571: AssertionError

graphql_api.tests.test_pull.TestPullRequestList test_bundle_analysis_sqlite_file_deleted

Stack Traces | 0.488s run time

self = &lt;graphql_api.tests.test_pull.TestPullRequestList testMethod=test_bundle_analysis_sqlite_file_deleted&gt;
get_storage_service = &lt;MagicMock name='get_appropriate_storage_service' id='139770357905216'&gt;

    @patch("graphql_api.dataloader.bundle_analysis.get_appropriate_storage_service")
    def test_bundle_analysis_sqlite_file_deleted(self, get_storage_service):
        os.system("rm -rf /tmp/bundle_analysis_*")
        storage = MemoryStorageService({})
        get_storage_service.return_value = storage
    
        parent_commit = CommitFactory(repository=self.repository)
        commit = CommitFactory(
            repository=self.repository,
            totals={"c": "12", "diff": [0, 0, 0, 0, 0, "14"]},
            parent_commit_id=parent_commit.commitid,
        )
    
        base_commit_report = CommitReportFactory(
            commit=parent_commit,
            report_type=CommitReport.ReportType.BUNDLE_ANALYSIS,
        )
        head_commit_report = CommitReportFactory(
            commit=commit, report_type=CommitReport.ReportType.BUNDLE_ANALYSIS
        )
    
        my_pull = PullFactory(
            repository=self.repository,
            title="test-pull-request",
            author=self.owner,
            head=head_commit_report.commit.commitid,
            compared_to=base_commit_report.commit.commitid,
            behind_by=23,
            behind_by_commit="1089nf898as-jdf09hahs09fgh",
        )
    
        with open("..../tests/samples/base_bundle_report.sqlite", "rb") as f:
            storage_path = StoragePaths.bundle_report.path(
                repo_key=ArchiveService.get_archive_hash(self.repository),
                report_key=base_commit_report.external_id,
            )
            storage.write_file(get_bucket_name(), storage_path, f)
    
        with open("..../tests/samples/head_bundle_report.sqlite", "rb") as f:
            storage_path = StoragePaths.bundle_report.path(
                repo_key=ArchiveService.get_archive_hash(self.repository),
                report_key=head_commit_report.external_id,
            )
            storage.write_file(get_bucket_name(), storage_path, f)
    
        query = """
            bundleAnalysisCompareWithBase {
                __typename
                ... on BundleAnalysisComparison {
                    bundleData {
                        size {
                            uncompress
                        }
                    }
                }
            }
        """
    
        pull = self.fetch_one_pull_request(my_pull.pullid, query)
    
&gt;       assert pull == {
            "bundleAnalysisCompareWithBase": {
                "__typename": "BundleAnalysisComparison",
                "bundleData": {
                    "size": {
                        "uncompress": 201720,
                    }
                },
            }
        }
E       AssertionError: assert {'bundleAnaly...thBase': None} == {'bundleAnaly...s': 201720}}}}
E         
E         Differing items:
E         {'bundleAnalysisCompareWithBase': None} != {'bundleAnalysisCompareWithBase': {'__typename': 'BundleAnalysisComparison', 'bundleData': {'size': {'uncompress': 201720}}}}
E         Use -v to get more diff

graphql_api/tests/test_pull.py:555: AssertionError

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

codecov · 2024-09-05T15:12:10Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.15%. Comparing base (4df7869) to head (b017428).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@               Coverage Diff                @@
##               main       #798        +/-   ##
================================================
+ Coverage   96.14000   96.15000   +0.01000     
================================================
  Files           812        812                
  Lines         18527      18576        +49     
================================================
+ Hits          17813      17862        +49     
  Misses          714        714

Flag	Coverage Δ
unit	`92.02% <100.00%> (+0.02%)`	⬆️
unit-latest-uploader	`92.02% <100.00%> (+0.02%)`	⬆️

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.

JerrySentry · 2024-09-05T17:36:07Z

graphql_api/types/bundle_analysis/base.py

+    before: Optional[str] = None,
+) -> Dict[str, object]:
+    # All filtered assets before pagination
+    assets = list(


A non-ideal way to perform pagination. It is still fetching all the assets (minus the ones filtered out) from the report and then return the requested blocks. Since fetching all assets is trivial for a given report we will live with this non optimized way. The major reason we want to paginate assets is because each asset object contains timeseries measurement data which is what's causing 99% of the load time. Paginating the assets mean we can lazy load the timeseries data on the UI. Note that fetching all assets in the first step doesn't begin the computation of the measurements data which means its fine.

ajay-sentry · 2024-09-06T18:49:12Z

graphql_api/types/bundle_analysis/base.py

+def resolve_assets_paginated(
+    bundle_report: BundleReport,
+    info: GraphQLResolveInfo,
+    ordering: AssetOrdering = AssetOrdering.SIZE,


should this and ordering_direction be optional as well since we're defaulting a value?

yes, good catch

wait I think it is currently ok, right now it should mean that ordering is of type AssetOrdering, but if that param is not passed in to the call it would be defaulted to AssetOrdering.SIZE. It should never be the case that ordering is null.

Fair enough, I can appreciate the defensive nature of having a fallback value here

graphql_api/types/bundle_analysis/base.py

ajay-sentry · 2024-09-06T18:51:53Z

graphql_api/types/bundle_analysis/base.py

+
+    # Slice edges by return size
+    if first is not None and first >= 0:
+        if len(assets) > first:


nit: we can use total_count var instead of len(assets) for all these references to len(assets) right?

total_count might not equal to len(assets) at this point of the code path. assets could have gotten sliced by the after/before if statements. total_count is for the grand total of results after filtering.

ahhh okay yeah I totally missed that previously, makes sense

ajay-sentry

looks good 👌

JerrySentry added 3 commits September 5, 2024 10:44

initial commit

02e3f9b

merge main

b6c52ae

update shared

83b1441

JerrySentry requested a review from a team as a code owner September 5, 2024 14:55

JerrySentry added 3 commits September 5, 2024 11:26

fix existing tests

6601223

add paginated assets unit tests

b2e7c2d

use a different test variant

126ea43

JerrySentry commented Sep 5, 2024

View reviewed changes

Merge branch 'main' into sep_04_page

1688ec4

ajay-sentry reviewed Sep 6, 2024

View reviewed changes

graphql_api/types/bundle_analysis/base.py Show resolved Hide resolved

ajay-sentry reviewed Sep 6, 2024

View reviewed changes

graphql_api/types/bundle_analysis/base.py Show resolved Hide resolved

ajay-sentry reviewed Sep 6, 2024

View reviewed changes

JerrySentry and others added 3 commits September 6, 2024 15:21

Merge branch 'main' into sep_04_page

a3ecdcb

Merge branch 'main' into sep_04_page

5b4cf67

review changes

f7717b6

ajay-sentry approved these changes Sep 9, 2024

View reviewed changes

Merge branch 'main' into sep_04_page

b017428

JerrySentry enabled auto-merge September 9, 2024 21:13

JerrySentry added this pull request to the merge queue Sep 9, 2024

Merged via the queue into main with commit f4e4a18 Sep 9, 2024
19 checks passed

JerrySentry deleted the sep_04_page branch September 9, 2024 21:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bundle analysis: add a paginated option to retrieve assets #798

bundle analysis: add a paginated option to retrieve assets #798

JerrySentry commented Sep 5, 2024 •

edited

Loading

codecov-staging bot commented Sep 5, 2024 •

edited by codecov-notifications bot

Loading

codecov-public-qa bot commented Sep 5, 2024

pytest

codecov-qa bot commented Sep 5, 2024

codecov bot commented Sep 5, 2024 •

edited

Loading

JerrySentry Sep 5, 2024

ajay-sentry Sep 6, 2024

JerrySentry Sep 6, 2024

JerrySentry Sep 6, 2024

ajay-sentry Sep 9, 2024

ajay-sentry Sep 6, 2024

JerrySentry Sep 6, 2024 •

edited

Loading

ajay-sentry Sep 9, 2024

ajay-sentry left a comment

bundle analysis: add a paginated option to retrieve assets #798

bundle analysis: add a paginated option to retrieve assets #798

Conversation

JerrySentry commented Sep 5, 2024 • edited Loading

Legal Boilerplate

codecov-staging bot commented Sep 5, 2024 • edited by codecov-notifications bot Loading

Codecov Report

codecov-public-qa bot commented Sep 5, 2024

❌ Failed Test Results:

pytest

codecov-qa bot commented Sep 5, 2024

❌ 5 Tests Failed:

codecov bot commented Sep 5, 2024 • edited Loading

Codecov Report

JerrySentry Sep 5, 2024

Choose a reason for hiding this comment

ajay-sentry Sep 6, 2024

Choose a reason for hiding this comment

JerrySentry Sep 6, 2024

Choose a reason for hiding this comment

JerrySentry Sep 6, 2024

Choose a reason for hiding this comment

ajay-sentry Sep 9, 2024

Choose a reason for hiding this comment

ajay-sentry Sep 6, 2024

Choose a reason for hiding this comment

JerrySentry Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

ajay-sentry Sep 9, 2024

Choose a reason for hiding this comment

ajay-sentry left a comment

Choose a reason for hiding this comment

JerrySentry commented Sep 5, 2024 •

edited

Loading

codecov-staging bot commented Sep 5, 2024 •

edited by codecov-notifications bot

Loading

codecov bot commented Sep 5, 2024 •

edited

Loading

JerrySentry Sep 6, 2024 •

edited

Loading