Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

Commit 8ded4cb

Browse files
JerrySentryJerrySentry
authored
Bundle Analysis: fix trends when no data in query range (#1267)
1 parent f717a2b commit 8ded4cb

File tree

5 files changed

+311
-17
lines changed

5 files changed

+311
-17
lines changed

‎graphql_api/actions/measurements.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ def measurements_by_ids(
1616
before: datetime,
1717
after: Optional[datetime] = None,
1818
branch: Optional[str] = None,
19-
) -> Dict[int, List[Dict[str, Any]]]:
19+
) -> Dict[Any, List[Dict[str, Any]]]:
2020
queryset = MeasurementSummary.agg_by(interval).filter(
2121
name=measurable_name,
2222
owner_id=repository.author_id,
@@ -38,7 +38,7 @@ def measurements_by_ids(
3838
)
3939

4040
# group by measurable_id
41-
measurements: Dict[int, List[Dict[str, Any]]] = {}
41+
measurements: Dict[Any, List[Dict[str, Any]]] = {}
4242
for measurement in queryset:
4343
measurable_id = measurement["measurable_id"]
4444
if measurable_id not in measurements:

‎graphql_api/tests/test_bundle_analysis_measurements.py‎

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3128,3 +3128,275 @@ def test_bundle_report_measurements_only_unknown(self, get_storage_service):
31283128
"name": "super",
31293129
},
31303130
}
3131+
3132+
@patch("graphql_api.dataloader.bundle_analysis.get_appropriate_storage_service")
3133+
def test_bundle_report_measurements_no_data_in_range(self, get_storage_service):
3134+
storage = MemoryStorageService({})
3135+
get_storage_service.return_value = storage
3136+
3137+
with open("./services/tests/samples/bundle_with_uuid.sqlite", "rb") as f:
3138+
storage_path = StoragePaths.bundle_report.path(
3139+
repo_key=ArchiveService.get_archive_hash(self.repo),
3140+
report_key=self.head_commit_report.external_id,
3141+
)
3142+
storage.write_file(get_bucket_name(), storage_path, f)
3143+
3144+
query = """
3145+
query FetchMeasurements(
3146+
$org: String!,
3147+
$repo: String!,
3148+
$commit: String!
3149+
$filters: BundleAnalysisMeasurementsSetFilters
3150+
$orderingDirection: OrderingDirection!
3151+
$interval: MeasurementInterval!
3152+
$before: DateTime!
3153+
$after: DateTime!
3154+
) {
3155+
owner(username: $org) {
3156+
repository(name: $repo) {
3157+
... on Repository {
3158+
commit(id: $commit) {
3159+
bundleAnalysis {
3160+
bundleAnalysisReport {
3161+
__typename
3162+
... on BundleAnalysisReport {
3163+
bundle(name: "super") {
3164+
name
3165+
measurements(
3166+
filters: $filters
3167+
orderingDirection: $orderingDirection
3168+
after: $after
3169+
interval: $interval
3170+
before: $before
3171+
){
3172+
assetType
3173+
name
3174+
measurements {
3175+
avg
3176+
min
3177+
max
3178+
timestamp
3179+
}
3180+
}
3181+
}
3182+
}
3183+
}
3184+
}
3185+
}
3186+
}
3187+
}
3188+
}
3189+
}
3190+
"""
3191+
3192+
# Test without using asset type filters
3193+
variables = {
3194+
"org": self.org.username,
3195+
"repo": self.repo.name,
3196+
"commit": self.commit.commitid,
3197+
"orderingDirection": "ASC",
3198+
"interval": "INTERVAL_1_DAY",
3199+
"after": "2024-06-08",
3200+
"before": "2024-06-09",
3201+
"filters": {},
3202+
}
3203+
data = self.gql_request(query, variables=variables)
3204+
commit = data["owner"]["repository"]["commit"]
3205+
3206+
assert commit["bundleAnalysis"]["bundleAnalysisReport"] == {
3207+
"__typename": "BundleAnalysisReport",
3208+
"bundle": {
3209+
"measurements": [
3210+
{
3211+
"assetType": "ASSET_SIZE",
3212+
"measurements": [
3213+
{
3214+
"avg": 4126.0,
3215+
"max": 4126.0,
3216+
"min": 4126.0,
3217+
"timestamp": "2024-06-08T00:00:00+00:00",
3218+
},
3219+
{
3220+
"avg": None,
3221+
"max": None,
3222+
"min": None,
3223+
"timestamp": "2024-06-09T00:00:00+00:00",
3224+
},
3225+
],
3226+
"name": "asset-*.js",
3227+
},
3228+
{
3229+
"assetType": "ASSET_SIZE",
3230+
"measurements": [],
3231+
"name": "asset-*.js",
3232+
},
3233+
{
3234+
"assetType": "ASSET_SIZE",
3235+
"measurements": [],
3236+
"name": "asset-*.js",
3237+
},
3238+
{
3239+
"assetType": "FONT_SIZE",
3240+
"measurements": [
3241+
{
3242+
"avg": 50.0,
3243+
"max": 50.0,
3244+
"min": 50.0,
3245+
"timestamp": "2024-06-08T00:00:00+00:00",
3246+
},
3247+
{
3248+
"avg": None,
3249+
"max": None,
3250+
"min": None,
3251+
"timestamp": "2024-06-09T00:00:00+00:00",
3252+
},
3253+
],
3254+
"name": None,
3255+
},
3256+
{
3257+
"assetType": "IMAGE_SIZE",
3258+
"measurements": [
3259+
{
3260+
"avg": 500.0,
3261+
"max": 500.0,
3262+
"min": 500.0,
3263+
"timestamp": "2024-06-08T00:00:00+00:00",
3264+
},
3265+
{
3266+
"avg": None,
3267+
"max": None,
3268+
"min": None,
3269+
"timestamp": "2024-06-09T00:00:00+00:00",
3270+
},
3271+
],
3272+
"name": None,
3273+
},
3274+
{
3275+
"assetType": "JAVASCRIPT_SIZE",
3276+
"measurements": [
3277+
{
3278+
"avg": 5708.0,
3279+
"max": 5708.0,
3280+
"min": 5708.0,
3281+
"timestamp": "2024-06-08T00:00:00+00:00",
3282+
},
3283+
{
3284+
"avg": None,
3285+
"max": None,
3286+
"min": None,
3287+
"timestamp": "2024-06-09T00:00:00+00:00",
3288+
},
3289+
],
3290+
"name": None,
3291+
},
3292+
{
3293+
"assetType": "REPORT_SIZE",
3294+
"measurements": [
3295+
{
3296+
"avg": 6263.0,
3297+
"max": 6263.0,
3298+
"min": 6263.0,
3299+
"timestamp": "2024-06-08T00:00:00+00:00",
3300+
},
3301+
{
3302+
"avg": None,
3303+
"max": None,
3304+
"min": None,
3305+
"timestamp": "2024-06-09T00:00:00+00:00",
3306+
},
3307+
],
3308+
"name": None,
3309+
},
3310+
{
3311+
"assetType": "STYLESHEET_SIZE",
3312+
"measurements": [
3313+
{
3314+
"avg": 5.0,
3315+
"max": 5.0,
3316+
"min": 5.0,
3317+
"timestamp": "2024-06-08T00:00:00+00:00",
3318+
},
3319+
{
3320+
"avg": None,
3321+
"max": None,
3322+
"min": None,
3323+
"timestamp": "2024-06-09T00:00:00+00:00",
3324+
},
3325+
],
3326+
"name": None,
3327+
},
3328+
{
3329+
"assetType": "UNKNOWN_SIZE",
3330+
"measurements": [
3331+
{
3332+
"avg": 0.0,
3333+
"max": 0.0,
3334+
"min": 0.0,
3335+
"timestamp": "2024-06-08T00:00:00+00:00",
3336+
},
3337+
{
3338+
"avg": None,
3339+
"max": None,
3340+
"min": None,
3341+
"timestamp": "2024-06-09T00:00:00+00:00",
3342+
},
3343+
],
3344+
"name": None,
3345+
},
3346+
],
3347+
"name": "super",
3348+
},
3349+
}
3350+
3351+
# Test with using asset type filters
3352+
variables = {
3353+
"org": self.org.username,
3354+
"repo": self.repo.name,
3355+
"commit": self.commit.commitid,
3356+
"orderingDirection": "ASC",
3357+
"interval": "INTERVAL_1_DAY",
3358+
"after": "2024-06-07",
3359+
"before": "2024-06-10",
3360+
"filters": {"assetTypes": "JAVASCRIPT_SIZE"},
3361+
}
3362+
data = self.gql_request(query, variables=variables)
3363+
commit = data["owner"]["repository"]["commit"]
3364+
3365+
assert commit["bundleAnalysis"]["bundleAnalysisReport"] == {
3366+
"__typename": "BundleAnalysisReport",
3367+
"bundle": {
3368+
"measurements": [
3369+
{
3370+
"assetType": "JAVASCRIPT_SIZE",
3371+
"measurements": [
3372+
{
3373+
"avg": 5708.0,
3374+
"max": 5708.0,
3375+
"min": 5708.0,
3376+
"timestamp": "2024-06-07T00:00:00+00:00",
3377+
},
3378+
{
3379+
"avg": None,
3380+
"max": None,
3381+
"min": None,
3382+
"timestamp": "2024-06-08T00:00:00+00:00",
3383+
},
3384+
{
3385+
"avg": None,
3386+
"max": None,
3387+
"min": None,
3388+
"timestamp": "2024-06-09T00:00:00+00:00",
3389+
},
3390+
{
3391+
"avg": 26708.0,
3392+
"max": 26708.0,
3393+
"min": 26708.0,
3394+
"timestamp": "2024-06-10T00:00:00+00:00",
3395+
},
3396+
],
3397+
"name": None,
3398+
},
3399+
],
3400+
"name": "super",
3401+
},
3402+
}

‎graphql_api/tests/test_pull.py‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
from datetime import datetime, timedelta
33
from unittest.mock import patch
44

5-
import pytest
65
from django.test import TestCase
76
from freezegun import freeze_time
87
from shared.api_archive.archive import ArchiveService
@@ -460,25 +459,21 @@ def test_with_complete_pull_request(self):
460459
"behindByCommit": "1089nf898as-jdf09hahs09fgh",
461460
}
462461

463-
@pytest.mark.skip(
464-
reason="Skipping due to https://github.com/codecov/engineering-team/issues/3358"
465-
)
466462
def test_compare_bundle_analysis_missing_reports(self):
467-
repository = RepositoryFactory(author=self.owner)
468463
head = CommitFactory(
469-
repository=repository,
464+
repository=self.repository,
470465
author=self.owner,
471466
commitid="cool-commit-id",
472467
totals={"c": "78.38", "diff": [0, 0, 0, 0, 0, "14"]},
473468
)
474469
compared_to = CommitFactory(
475-
repository=repository,
470+
repository=self.repository,
476471
author=self.owner,
477472
commitid="blah",
478473
)
479474

480475
my_pull = PullFactory(
481-
repository=repository,
476+
repository=self.repository,
482477
author=self.owner,
483478
head=head.commitid,
484479
compared_to=compared_to.commitid,

‎services/bundle_analysis.py‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ def __init__(
454454
@sentry_sdk.trace
455455
def _compute_measurements(
456456
self, measurable_name: str, measurable_ids: List[str]
457-
) -> Dict[int, List[Dict[str, Any]]]:
457+
) -> Dict[str, List[Dict[str, Any]]]:
458458
all_measurements = measurements_by_ids(
459459
repository=self.repository,
460460
measurable_name=measurable_name,
@@ -463,11 +463,13 @@ def _compute_measurements(
463463
after=self.after,
464464
before=self.before,
465465
branch=self.branch,
466-
)
466+
) or {measurable_ids[0]: []}
467467

468468
# Carry over previous available value for start date if its value is null
469469
for measurable_id, measurements in all_measurements.items():
470-
if self.after is not None and measurements[0]["timestamp_bin"] > self.after:
470+
if self.after is not None and (
471+
not measurements or measurements[0]["timestamp_bin"] > self.after
472+
):
471473
carryover_measurement = measurements_last_uploaded_before_start_date(
472474
owner_id=self.repository.author.ownerid,
473475
repo_id=self.repository.repoid,
@@ -481,8 +483,10 @@ def _compute_measurements(
481483
# If there isn't any measurements before the start date range, measurements will be untouched
482484
if carryover_measurement:
483485
value = Decimal(carryover_measurement[0]["value"])
484-
carryover = dict(measurements[0])
485-
carryover["timestamp_bin"] = self.after
486+
carryover = dict(measurements[0]) if measurements else {}
487+
carryover["timestamp_bin"] = self.after.replace(
488+
hour=0, minute=0, second=0, microsecond=0
489+
)
486490
carryover["min"] = value
487491
carryover["max"] = value
488492
carryover["avg"] = value

0 commit comments

Comments
 (0)