create utils for accessing testrun timescale models #1078

joseph-sentry · 2025-02-10T21:25:58Z

this PR creates some utility functions for inserting and aggregating testrun data in timescale

transaction=True is required for the test_get_testrun_summary for the continuous aggregates to populate the data that is expected to be in the summary for the test to pass

depends on: codecov/shared#508

codecov · 2025-02-10T21:30:05Z

Codecov Report

Attention: Patch coverage is 97.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 97.77%. Comparing base (4abcb17) to head (b23aa8b).

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/test_analytics/ta_timeseries.py	95.71%	3 Missing ⚠️
services/test_analytics/utils.py	76.92%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1078      +/-   ##
==========================================
- Coverage   97.78%   97.77%   -0.01%     
==========================================
  Files         443      446       +3     
  Lines       36565    36765     +200     
==========================================
+ Hits        35754    35948     +194     
- Misses        811      817       +6

Flag	Coverage Δ
integration	`42.85% <76.50%> (+0.18%)`	⬆️
unit	`90.49% <63.50%> (-0.15%)`	⬇️

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-qa · 2025-02-10T21:30:27Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
1	1	0	0

View the top 1 failed test(s) by shortest run time

services/tests/test_ta_timeseries/test_ta_timeseries.py::services.tests.test_ta_timeseries.test_ta_timeseries

Stack Traces | 0s run time

ImportError while importing test module '.../tests/test_ta_timeseries/test_ta_timeseries.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
.../local/lib/python3.13/importlib/__init__.py:88: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
.../tests/test_ta_timeseries/test_ta_timeseries.py:6: in <module>
    from shared.django_apps.timeseries.models import Testrun
E   ImportError: cannot import name 'Testrun' from 'shared.django_apps.timeseries.models' (.../local/lib/python3.13.../django_apps/timeseries/models.py)

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

codecov-public-qa · 2025-02-10T21:30:39Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
1	1	0	0

View the top 1 failed tests by shortest run time

services/tests/test_ta_timeseries/test_ta_timeseries.py::::services.tests.test_ta_timeseries.test_ta_timeseries

Stack Traces | 0s run time

No failure message available

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

github-actions · 2025-02-10T21:30:58Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
2	2	0	0

View the top 2 failed tests by shortest run time

services.tests.test_ta_timeseries.test_ta_timeseries

Stack Traces | 0.000s run time

No failure message available

services.tests.test_ta_timeseries.test_ta_timeseries

Stack Traces | 0.000s run time

No failure message available

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

Swatinem

finally some tests that you can actually run locally :-)
though my main concern is whether the aggregates are actually live, see the specific comment.

Swatinem · 2025-02-12T10:50:23Z

services/ta_timeseries.py

+                upload_id=upload_id,
+            )
+        )
+    Testrun.objects.bulk_create(testruns_to_create)


I would use the batch_size parameter here. Otherwise you would end up with "unique queries" for each of the N number of tests you have.

Its really unfortunate that SQL does not have a proper batch create query for these purposes.

Swatinem · 2025-02-12T10:50:47Z

services/ta_timeseries.py

+    repo_id: int | None,
+    commit_sha: str | None,
+    branch: str | None,
+    upload_id: int | None,
+    flags: list[str] | None,
+    parsing_info: test_results_parser.ParsingInfo,
+    flaky_test_ids: set[bytes] | None = None,


why are all these optionally None?

they shouldn't be, except for maybe flags and flaky_test_ids, i'll fix

Swatinem · 2025-02-12T10:52:19Z

services/ta_timeseries.py

+                "test_id": bytes(test_id),
+                "flags_hash": bytes(flags_hash),


is the test_id as returned by the database hex-encoded? in that case casting this to bytes does not make any sense?

it's a memoryview, which is basically just a pointer, and to dereference we must wrap it in bytes

Swatinem · 2025-02-12T10:57:10Z

services/ta_timeseries.py

+    with connections["timeseries"].cursor() as cursor:
+        cursor.execute(
+            "UPDATE timeseries_testrun SET outcome = %s WHERE timestamp = %s AND test_id = %s AND flags_hash = %s",
+            ["flaky_failure", timestamp, test_id, flags_hash],
+        )


I would use the ORM instead of a raw query here.

Swatinem · 2025-02-12T10:58:05Z

services/ta_timeseries.py

+
+
+def get_testrun_summary(
+    repo_id: int, interval: Interval, branch: str | None = None


the branch parameter is unused

Swatinem · 2025-02-12T11:00:11Z

services/ta_timeseries.py

+) -> list[TestrunSummary]:
+    timestamp_bin = datetime.now() - timedelta(days=interval.value)
+    return list(
+        TestrunSummary.objects.filter(repo_id=repo_id, timestamp_bin__gte=timestamp_bin)


both these functions return the daily buckets to python, instead of aggregating across the interval within the DB, is that really the intention here?

we can discuss this more, but I thought we wanted to use the same idea from your binary format where the cached file has individual rows for each day so that we can prune out of date rows when reading from the cache file

if we're skipping the cached file and doing this query on the fly each time, then we should aggregate within the DB

but i think we should compare those 2 alternatives in prod with real data to see which one is faster

Swatinem · 2025-02-12T11:03:44Z

services/tests/test_ta_timeseries/test_ta_timeseries.py

+    testruns = get_testruns_for_flake_detection(
+        1,
+        {
+            calc_test_id("flaky_test_name", "test_classname", "test_suite"),


the weird autoformatting gives us a hint to make this a local variable :-D

Swatinem · 2025-02-12T11:10:59Z

services/tests/test_ta_timeseries/test_ta_timeseries.py

+                'timeseries_testrun_summary_1day',
+                start_offset => '7 days',
+                end_offset => NULL,
+                schedule_interval => INTERVAL '1 milliseconds'


this is a bit weird. given that the default is set to one day, does that mean these aggregates are only updated once a day, and if you query these during the day, they would not give you any result for "today"?

I think the "continuous aggregation" should indeed be continuous and live.
meaning that when you INSERT into the testruns table, the materialized view with the aggregate should yield aggregates for that immediately. without having to mess with some kind of setting, and without having to put a sleep into the test.

so from what i understand about timescale, the continuous aggregates, aren't actually continuous, they're aggregated using basically a cron job, and we can tweak the schedule of the cron job based on this policy. There is a setting where on read, it will get all the data that has been materialized by the cron job, AND get all the unmaterialized data, materialize it on the fly, and merge it into the results but that would obviously be more expensive

Swatinem · 2025-02-12T11:12:19Z

services/tests/test_ta_timeseries/test_ta_timeseries.py

+    assert summaries[0].fail_count == 1
+    assert summaries[0].flaky_fail_count == 0
+    assert summaries[0].skip_count == 0
+    assert summaries[0].flags == [["flag1"], ["flag2"]]


interesting. so the aggregates are aggregating across all the flags? I thought you still wanted to filter these 🤔

this is a direct port of what we're showing in the UI right now, to change this would require a product-focused discussion

relies on shared changes from: codecov/shared#508 this implements the following TA functionality using data from Timescale: - PR comment summary - PR comment failure details - PR flake set - Flake detection relevant testruns - Flake detection relevant flakes - All branches testrun summary - Main branches testrun summary - Feature branch testrun summary Also moves the flag id calculation to a new file

codecov-notifications · 2025-03-03T20:50:40Z

Codecov Report

Attention: Patch coverage is 97.00000% with 6 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/test_analytics/ta_timeseries.py	95.71%	3 Missing ⚠️
services/test_analytics/utils.py	76.92%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

joseph-sentry marked this pull request as draft February 11, 2025 14:27

joseph-sentry force-pushed the joseph/ta-timeseries branch from 7f8247d to f21ee54 Compare February 11, 2025 21:35

joseph-sentry mentioned this pull request Feb 11, 2025

Use TA Timeseries models in TA processor task #1084

Draft

Swatinem reviewed Feb 12, 2025

View reviewed changes

joseph-sentry force-pushed the joseph/ta-timeseries branch from f21ee54 to 4458704 Compare February 28, 2025 22:48

This was referenced Feb 28, 2025

chore: remove previous code #1116

Merged

feat: new TA processor implementation #1117

Draft

Swatinem approved these changes Mar 3, 2025

View reviewed changes

joseph-sentry force-pushed the joseph/ta-timeseries branch from 4458704 to b23aa8b Compare March 3, 2025 20:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create utils for accessing testrun timescale models #1078

create utils for accessing testrun timescale models #1078

joseph-sentry commented Feb 10, 2025

codecov bot commented Feb 10, 2025 •

edited

Loading

codecov-qa bot commented Feb 10, 2025 •

edited

Loading

codecov-public-qa bot commented Feb 10, 2025

github-actions bot commented Feb 10, 2025

Swatinem left a comment

Swatinem Feb 12, 2025

Swatinem Feb 12, 2025

joseph-sentry Mar 4, 2025

Swatinem Feb 12, 2025

joseph-sentry Mar 4, 2025

Swatinem Feb 12, 2025

Swatinem Feb 12, 2025

Swatinem Feb 12, 2025

joseph-sentry Mar 4, 2025

Swatinem Feb 12, 2025

Swatinem Feb 12, 2025

joseph-sentry Mar 4, 2025

Swatinem Feb 12, 2025

joseph-sentry Mar 4, 2025

codecov-notifications bot commented Mar 3, 2025 •

edited

Loading



		def get_testrun_summary(
		repo_id: int, interval: Interval, branch: str \| None = None

create utils for accessing testrun timescale models #1078

Are you sure you want to change the base?

create utils for accessing testrun timescale models #1078

Conversation

joseph-sentry commented Feb 10, 2025

codecov bot commented Feb 10, 2025 • edited Loading

Codecov Report

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

❌ 1 Tests Failed:

codecov-public-qa bot commented Feb 10, 2025

❌ 1 Tests Failed:

github-actions bot commented Feb 10, 2025

❌ 2 Tests Failed:

Swatinem left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-notifications bot commented Mar 3, 2025 • edited Loading

Codecov Report

codecov bot commented Feb 10, 2025 •

edited

Loading

codecov-qa bot commented Feb 10, 2025 •

edited

Loading

codecov-notifications bot commented Mar 3, 2025 •

edited

Loading