add percent changes to test results and flakes aggregates #829

joseph-sentry · 2024-09-16T20:56:33Z

This adds PercentChange fields to both the TestResultsAggregates
and the FlakeAggregates. These work by getting the aggregates
for the previous historical period and comparing them with the
ones for the current historical period and getting a percentage difference.

codecov-staging · 2024-09-16T21:06:43Z

Codecov Report

Attention: Patch coverage is 98.71795% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
utils/test_results.py	97.22%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

codecov-qa · 2024-09-16T21:06:50Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
2339	2	2337	6

View the top 2 failed tests by shortest run time

graphql_api.tests.test_test_results_headers.TestResultTestCase test_fetch_test_result_total_runtime

Stack Traces | 0.419s run time

self = &lt;graphql_api.tests.test_test_results_headers.TestResultTestCase testMethod=test_fetch_test_result_total_runtime&gt;

    def test_fetch_test_result_total_runtime(self) -&gt; None:
        query = """
            query {
               owner(username: "%s") {
                    repository(name: "%s") {
                        ... on Repository {
                            testResultsAggregates {
                                totalRunTime
                            }
                        }
                    }
                 }
            }
        """ % (self.owner.username, self.repository.name)
    
&gt;       result = self.gql_request(query, owner=self.owner)

graphql_api/tests/test_test_results_headers.py:48: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = &lt;graphql_api.tests.test_test_results_headers.TestResultTestCase testMethod=test_fetch_test_result_total_runtime&gt;
query = '\n            query {\n               owner(username: "randomOwner") {\n                    repository(name: "address...                      }\n                        }\n                    }\n                 }\n            }\n        '
provider = 'gh', owner = &lt;Owner: Owner&lt;github/randomOwner&gt;&gt;, variables = None
with_errors = False, okta_signed_in_accounts = [], impersonate_owner = False

    def gql_request(
        self,
        query,
        provider="gh",
        owner=None,
        variables=None,
        with_errors=False,
        okta_signed_in_accounts=[],
        impersonate_owner=False,
    ):
        url = f"/graphql/{provider}"
    
        if owner:
            self.client = Client()
    
            if impersonate_owner:
                staff_owner = OwnerFactory(
                    name="staff_user", service="github", user=UserFactory(is_staff=True)
                )
                self.client.cookies = SimpleCookie({"staff_user": owner.pk})
                self.client.force_login_owner(staff_owner)
            else:
                self.client.force_login_owner(owner)
    
            if okta_signed_in_accounts:
                session = self.client.session
                session[OKTA_SIGNED_IN_ACCOUNTS_SESSION_KEY] = okta_signed_in_accounts
                session.save()
    
        response = self.client.post(
            url,
            {"query": query, "variables": variables or {}},
            content_type="application/json",
        )
&gt;       return response.json() if with_errors else response.json()["data"]
E       KeyError: 'data'

graphql_api/tests/helper.py:44: KeyError

graphql_api.tests.test_test_results_headers.TestResultTestCase test_fetch_test_result_slowest_tests_runtime

Stack Traces | 0.434s run time

self = &lt;graphql_api.tests.test_test_results_headers.TestResultTestCase testMethod=test_fetch_test_result_slowest_tests_runtime&gt;

    def test_fetch_test_result_slowest_tests_runtime(self) -&gt; None:
        query = """
            query {
               owner(username: "%s") {
                    repository(name: "%s") {
                        ... on Repository {
                            testResultsAggregates {
                                slowestTestsRunTime
                            }
                        }
                    }
                 }
            }
        """ % (self.owner.username, self.repository.name)
    
&gt;       result = self.gql_request(query, owner=self.owner)

graphql_api/tests/test_test_results_headers.py:71: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = &lt;graphql_api.tests.test_test_results_headers.TestResultTestCase testMethod=test_fetch_test_result_slowest_tests_runtime&gt;
query = '\n            query {\n               owner(username: "randomOwner") {\n                    repository(name: "so") {\...                      }\n                        }\n                    }\n                 }\n            }\n        '
provider = 'gh', owner = &lt;Owner: Owner&lt;github/randomOwner&gt;&gt;, variables = None
with_errors = False, okta_signed_in_accounts = [], impersonate_owner = False

    def gql_request(
        self,
        query,
        provider="gh",
        owner=None,
        variables=None,
        with_errors=False,
        okta_signed_in_accounts=[],
        impersonate_owner=False,
    ):
        url = f"/graphql/{provider}"
    
        if owner:
            self.client = Client()
    
            if impersonate_owner:
                staff_owner = OwnerFactory(
                    name="staff_user", service="github", user=UserFactory(is_staff=True)
                )
                self.client.cookies = SimpleCookie({"staff_user": owner.pk})
                self.client.force_login_owner(staff_owner)
            else:
                self.client.force_login_owner(owner)
    
            if okta_signed_in_accounts:
                session = self.client.session
                session[OKTA_SIGNED_IN_ACCOUNTS_SESSION_KEY] = okta_signed_in_accounts
                session.save()
    
        response = self.client.post(
            url,
            {"query": query, "variables": variables or {}},
            content_type="application/json",
        )
&gt;       return response.json() if with_errors else response.json()["data"]
E       KeyError: 'data'

graphql_api/tests/helper.py:44: KeyError

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

codecov-public-qa · 2024-09-16T21:06:54Z

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

❌ Failed Test Results:

Completed 2345 tests with 2 failed, 2337 passed and 6 skipped.

View the full list of failed tests

pytest

Class name: graphql_api.tests.test_test_results_headers.TestResultTestCase
Test name: test_fetch_test_result_slowest_tests_runtime

self = <graphql_api.tests.test_test_results_headers.TestResultTestCase testMethod=test_fetch_test_result_slowest_tests_runtime>

    def test_fetch_test_result_slowest_tests_runtime(self) -> None:
        query = """
            query {
               owner(username: "%s") {
                    repository(name: "%s") {
                        ... on Repository {
                            testResultsAggregates {
                                slowestTestsRunTime
                            }
                        }
                    }
                 }
            }
        """ % (self.owner.username, self.repository.name)
    
>       result = self.gql_request(query, owner=self.owner)

graphql_api/tests/test_test_results_headers.py:71: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <graphql_api.tests.test_test_results_headers.TestResultTestCase testMethod=test_fetch_test_result_slowest_tests_runtime>
query = '\n            query {\n               owner(username: "randomOwner") {\n                    repository(name: "so") {\...                      }\n                        }\n                    }\n                 }\n            }\n        '
provider = 'gh', owner = <Owner: Owner<github/randomOwner>>, variables = None
with_errors = False, okta_signed_in_accounts = [], impersonate_owner = False

    def gql_request(
        self,
        query,
        provider="gh",
        owner=None,
        variables=None,
        with_errors=False,
        okta_signed_in_accounts=[],
        impersonate_owner=False,
    ):
        url = f"/graphql/{provider}"
    
        if owner:
            self.client = Client()
    
            if impersonate_owner:
                staff_owner = OwnerFactory(
                    name="staff_user", service="github", user=UserFactory(is_staff=True)
                )
                self.client.cookies = SimpleCookie({"staff_user": owner.pk})
                self.client.force_login_owner(staff_owner)
            else:
                self.client.force_login_owner(owner)
    
            if okta_signed_in_accounts:
                session = self.client.session
                session[OKTA_SIGNED_IN_ACCOUNTS_SESSION_KEY] = okta_signed_in_accounts
                session.save()
    
        response = self.client.post(
            url,
            {"query": query, "variables": variables or {}},
            content_type="application/json",
        )
>       return response.json() if with_errors else response.json()["data"]
E       KeyError: 'data'

graphql_api/tests/helper.py:44: KeyError

Class name: graphql_api.tests.test_test_results_headers.TestResultTestCase
Test name: test_fetch_test_result_total_runtime

self = <graphql_api.tests.test_test_results_headers.TestResultTestCase testMethod=test_fetch_test_result_total_runtime>

    def test_fetch_test_result_total_runtime(self) -> None:
        query = """
            query {
               owner(username: "%s") {
                    repository(name: "%s") {
                        ... on Repository {
                            testResultsAggregates {
                                totalRunTime
                            }
                        }
                    }
                 }
            }
        """ % (self.owner.username, self.repository.name)
    
>       result = self.gql_request(query, owner=self.owner)

graphql_api/tests/test_test_results_headers.py:48: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <graphql_api.tests.test_test_results_headers.TestResultTestCase testMethod=test_fetch_test_result_total_runtime>
query = '\n            query {\n               owner(username: "randomOwner") {\n                    repository(name: "address...                      }\n                        }\n                    }\n                 }\n            }\n        '
provider = 'gh', owner = <Owner: Owner<github/randomOwner>>, variables = None
with_errors = False, okta_signed_in_accounts = [], impersonate_owner = False

    def gql_request(
        self,
        query,
        provider="gh",
        owner=None,
        variables=None,
        with_errors=False,
        okta_signed_in_accounts=[],
        impersonate_owner=False,
    ):
        url = f"/graphql/{provider}"
    
        if owner:
            self.client = Client()
    
            if impersonate_owner:
                staff_owner = OwnerFactory(
                    name="staff_user", service="github", user=UserFactory(is_staff=True)
                )
                self.client.cookies = SimpleCookie({"staff_user": owner.pk})
                self.client.force_login_owner(staff_owner)
            else:
                self.client.force_login_owner(owner)
    
            if okta_signed_in_accounts:
                session = self.client.session
                session[OKTA_SIGNED_IN_ACCOUNTS_SESSION_KEY] = okta_signed_in_accounts
                session.save()
    
        response = self.client.post(
            url,
            {"query": query, "variables": variables or {}},
            content_type="application/json",
        )
>       return response.json() if with_errors else response.json()["data"]
E       KeyError: 'data'

graphql_api/tests/helper.py:44: KeyError

codecov · 2024-09-16T21:07:52Z

Codecov Report

Attention: Patch coverage is 98.71795% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.31%. Comparing base (acf749e) to head (f73c190).
Report is 5 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
utils/test_results.py	97.22%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #829      +/-   ##
==========================================
+ Coverage   96.28%   96.31%   +0.03%     
==========================================
  Files         818      818              
  Lines       18818    18975     +157     
==========================================
+ Hits        18118    18275     +157     
  Misses        700      700

Flag	Coverage Δ
unit	`92.62% <98.71%> (+0.06%)`	⬆️
unit-latest-uploader	`92.62% <98.71%> (+0.06%)`	⬆️

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.

ajay-sentry · 2024-09-17T18:51:16Z

graphql_api/types/flake_aggregates/__init__.py

+
+flake_aggregates = ariadne_load_local_graphql(__file__, "flake_aggregates.graphql")
+
+__all__ = ["get_current_license", "flake_aggregates_bindable"]


do we need get_current_license here?

ajay-sentry · 2024-09-17T18:51:44Z

graphql_api/types/test_results_aggregates/__init__.py

+    __file__, "test_results_aggregates.graphql"
+)
+
+__all__ = ["get_current_license", "test_results_aggregates_bindable"]


Similar story with if we need get_current_license

ajay-sentry · 2024-09-17T18:52:48Z

graphql_api/types/flake_aggregates/flake_aggregates.py

+    return obj["flake_rate"]
+
+
+@flake_aggregates_bindable.field("flakeCountPercentChange")


nit: would personally organize these in "bunches" like flakeCount together and then flakeRate together vs. all the counts and all the %'s

I could see either approach tho

ajay-sentry · 2024-09-17T18:53:20Z

graphql_api/types/flake_aggregates/flake_aggregates.py

+
+
+@flake_aggregates_bindable.field("flakeRatePercentChange")
+def resolve_flake_rate_percent_change(obj, _) -> float | None:


in what situation would we return None for these? Vs. returning 0

ajay-sentry · 2024-09-17T18:54:22Z

graphql_api/types/repository/repository.py

+    repository: Repository,
+    info: GraphQLResolveInfo,
+):
+    queryset = await sync_to_async(generate_test_results_aggregates)(


nit: can you just do a return await here for both of these resolvers?

ajay-sentry · 2024-09-17T18:59:02Z

graphql_api/types/test_results/test_results.py

@@ -25,6 +25,16 @@ def resolve_failure_rate(test, info) -> float | None:
    return test["failure_rate"]


+@test_result_bindable.field("flakeRate")
+def resolve_flake_rate(test, info) -> float | None:


when would we want these to return None vs. 0?

ajay-sentry · 2024-09-17T21:39:52Z

graphql_api/types/test_results_aggregates/test_results_aggregates.py

+
+@test_results_aggregates_bindable.field("slowestTestsRunTimePercentChange")
+def resolve_slowest_tests_run_time_percent_change(obj, _) -> float | None:
+    return obj.get("slowest_tests_duration_percent_change")


nit: why is this called duration vs. runTime like the other? and to make resolver name

ajay-sentry · 2024-09-17T21:40:22Z

graphql_api/types/test_results_aggregates/test_results_aggregates.py

+
+
+@test_results_aggregates_bindable.field("totalRunTimePercentChange")
+def resolve_run_time_percent_change(obj, _) -> float | None:


Similarly when would these be None vs. 0?

ajay-sentry · 2024-09-17T21:44:45Z

utils/test_results.py

-    commits_where_fail: list[str] | None
-    average_duration: float | None
+def slow_test_threshold(num_tests: int):
+    threshold = floor(num_tests * (100 - SLOW_TEST_PERCENTILE) * 0.01)


nit: / 100 is a little easier to understand what we're doing here vs. * .01

ajay-sentry · 2024-09-17T21:50:21Z

utils/test_results.py

+def slow_test_threshold(num_tests: int):
+    threshold = floor(num_tests * (100 - SLOW_TEST_PERCENTILE) * 0.01)
+    if threshold == 0:
+        if num_tests < (1 / ((100 - SLOW_TEST_PERCENTILE) * 0.01)):


Is this line needed? When would this statement be false if we're taking the floor already on line 30

ajay-sentry · 2024-09-17T21:50:53Z

utils/test_results.py

    repoid: int,
    branch: str | None = None,
    history: dt.timedelta | None = None,
+    parameter: GENERATE_TEST_RESULT_PARAM | None = None,
 ) -> QuerySet:
    """
    Function that retrieves aggregated information about all tests in a given repository, for a given time range, optionally filtered by branch name.
    The fields it calculates are: the test failure rate, commits where this test failed, and average duration of the test.


as well as last duration, flake rate, etc

ajay-sentry · 2024-09-17T21:51:30Z

utils/test_results.py

-            branch=branch
-        )
+        totals = totals.filter(branch=branch)
+    print(parameter)


Reminder prints

ajay-sentry · 2024-09-17T21:56:24Z

utils/test_results.py

+            test_ids = [flake.test_id for flake in flakes]
+
+            totals = totals.filter(test_id__in=test_ids)
+        case GENERATE_TEST_RESULT_PARAM.FAILED:


Question: do cases in python automatically have break statements built in?

ajay-sentry · 2024-09-17T21:58:01Z

utils/test_results.py

+
+
+def percent_diff(a: int | float, b: int | float) -> int | float:
+    c = (a - b) / b * 100


nit: just return this instead of doing variable assignment

ajay-sentry · 2024-09-17T21:58:25Z

utils/test_results.py

+) -> dict[str, int | float]:
+    diff_dict = {}
+
+    for s in ls:


nit: list comprehension

ajay-sentry · 2024-09-17T22:06:30Z

utils/test_results.py

+) -> dict[str, float | int] | None:
+    repo = Repository.objects.get(repoid=repoid)
+    time_ago = (
+        (dt.datetime.now(dt.UTC) - history) if history is not None else thirty_days_ago


we can simplify these if else's a bit if we set History to be a const dependent on it's value being passed in OR dt.timedelta(days=30)

so then we can just do time_ago = (
(dt.datetime.now(dt.UTC) - history) and double_time_ago = time_ago -
history

WIth something like history = history or dt.timedelta(days=30)

ajay-sentry · 2024-09-17T22:07:23Z

utils/test_results.py

+        test_rollups = test_rollups.filter(date__lte=until.date())
+
+    if len(test_rollups) == 0:
+        assert 0 == 2


reminder remove assert

This adds PercentChange fields to both the TestResultsAggregates and the FlakeAggregates. These work by getting the aggregates for the previous historical period and comparing them with the ones for the current historical period and getting a percentage difference.

ajay-sentry

looks goooood

ajay-sentry reviewed Sep 17, 2024

View reviewed changes

utils/test_results.py Outdated

branch=branch

)

totals = totals.filter(branch=branch)

print(parameter)

Copy link

Contributor

ajay-sentry Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder prints

ajay-sentry reviewed Sep 17, 2024

View reviewed changes

utils/test_results.py Outdated

) -> dict[str, int | float]:

diff_dict = {}

for s in ls:

Copy link

Contributor

ajay-sentry Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: list comprehension

ajay-sentry reviewed Sep 17, 2024

View reviewed changes

joseph-sentry mentioned this pull request Oct 1, 2024

[TA] UI Support Work codecov/engineering-team#2528

Closed

joseph-sentry force-pushed the joseph/percentage-change branch 2 times, most recently from d0fdc20 to a57b8f3 Compare October 3, 2024 20:59

joseph-sentry added 4 commits October 3, 2024 17:02

fix: tests

c209c88

fix: tests

c78dfb9

fix: get rid of print stmts

2a2d550

joseph-sentry force-pushed the joseph/percentage-change branch from a57b8f3 to 2a2d550 Compare October 3, 2024 21:07

joseph-sentry marked this pull request as ready for review October 4, 2024 13:38

joseph-sentry requested a review from a team as a code owner October 4, 2024 13:38

joseph-sentry added 5 commits October 4, 2024 10:54

fix: handle edge case, add tests for flakeAggregates

1407d22

refactor: clean up the code and rename some vars

311d754

chore: make lint

b29215f

some more refactoring and renaming of vars

d31f981

chore: make lint

9867eb4

ajay-sentry approved these changes Oct 4, 2024

View reviewed changes

test: fix tests

f73c190

joseph-sentry added this pull request to the merge queue Oct 4, 2024

Merged via the queue into main with commit 16557ec Oct 4, 2024
18 of 19 checks passed

joseph-sentry deleted the joseph/percentage-change branch October 4, 2024 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add percent changes to test results and flakes aggregates #829

add percent changes to test results and flakes aggregates #829

joseph-sentry commented Sep 16, 2024

codecov-staging bot commented Sep 16, 2024 •

edited by codecov-notifications bot

Loading

codecov-qa bot commented Sep 16, 2024 •

edited

Loading

codecov-public-qa bot commented Sep 16, 2024 •

edited

Loading

pytest

codecov bot commented Sep 16, 2024 •

edited

Loading

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry Sep 17, 2024

ajay-sentry left a comment


		flake_aggregates = ariadne_load_local_graphql(__file__, "flake_aggregates.graphql")

		__all__ = ["get_current_license", "flake_aggregates_bindable"]

		return obj["flake_rate"]


		@flake_aggregates_bindable.field("flakeCountPercentChange")



		@flake_aggregates_bindable.field("flakeRatePercentChange")
		def resolve_flake_rate_percent_change(obj, _) -> float \| None:



		@test_results_aggregates_bindable.field("totalRunTimePercentChange")
		def resolve_run_time_percent_change(obj, _) -> float \| None:



		def percent_diff(a: int \| float, b: int \| float) -> int \| float:
		c = (a - b) / b * 100

add percent changes to test results and flakes aggregates #829

add percent changes to test results and flakes aggregates #829

Conversation

joseph-sentry commented Sep 16, 2024

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

Codecov Report

codecov-qa bot commented Sep 16, 2024 • edited Loading

❌ 2 Tests Failed:

codecov-public-qa bot commented Sep 16, 2024 • edited Loading

❌ Failed Test Results:

pytest

codecov bot commented Sep 16, 2024 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajay-sentry left a comment

Choose a reason for hiding this comment

codecov-staging bot commented Sep 16, 2024 •

edited by codecov-notifications bot

Loading

codecov-qa bot commented Sep 16, 2024 •

edited

Loading

codecov-public-qa bot commented Sep 16, 2024 •

edited

Loading

codecov bot commented Sep 16, 2024 •

edited

Loading