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

perf: Fix TestResults SQL query to not take as long #880

Merged
merged 11 commits into from
Oct 18, 2024
Merged

perf: Fix TestResults SQL query to not take as long #880

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b731bdf
perf: replace previous query for GQL test results
joseph-sentry Oct 10, 2024
811a290
fix: add some more tests
joseph-sentry Oct 10, 2024
de9265f
fix: rename some of the SQL aliases
joseph-sentry Oct 10, 2024
13fd39d
fix: use dict for params for test result sql query
joseph-sentry Oct 15, 2024
9c16f21
Merge branch 'main' of github.com:codecov/codecov-api into joseph/fix…
joseph-sentry Oct 15, 2024
e3f5582
fix: we're going to be coalescing the name and computed_name in the q…
joseph-sentry Oct 17, 2024
f57a0f3
fix: add cursor logic to test analytics query
joseph-sentry Oct 17, 2024
eedcb63
Merge branch 'main' of github.com:codecov/codecov-api into joseph/fix…
joseph-sentry Oct 17, 2024
2c7c33a
fix: fix some errors that happened during the merge
joseph-sentry Oct 17, 2024
2d7ef05
test: fix one more test
joseph-sentry Oct 17, 2024
96ad471
fix: add some clarifying comments / address feedback
joseph-sentry Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 206 additions & 5 deletions graphql_api/tests/test_test_analytics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import datetime
from base64 import b64encode

from django.test import TransactionTestCase
from shared.django_apps.reports.tests.factories import FlakeFactory
Expand All @@ -17,6 +18,10 @@
from .helper import GraphQLTestHelper


def base64_encode_string(x: str) -> str:
return b64encode(x.encode()).decode("utf-8")


class TestAnalyticsTestCase(GraphQLTestHelper, TransactionTestCase):
def setUp(self) -> None:
self.owner = OwnerFactory(username="codecov-user")
Expand Down Expand Up @@ -119,6 +124,28 @@ def test_branch_filter_on_test_results(self) -> None:
)
assert res["testResults"] == {"edges": [{"node": {"name": test.name}}]}

def test_interval_filter_on_test_results(self) -> None:
repo = RepositoryFactory(author=self.owner, active=True, private=True)
test = TestFactory(repository=repo)
test2 = TestFactory(repository=repo)
_ = DailyTestRollupFactory(
test=test,
date=datetime.datetime.now() - datetime.timedelta(days=7),
repoid=repo.repoid,
branch="main",
)
_ = DailyTestRollupFactory(
test=test2,
date=datetime.datetime.now(),
repoid=repo.repoid,
branch="feature",
)
res = self.fetch_test_analytics(
repo.name,
"""testResults(filters: { interval: INTERVAL_1_DAY }) { edges { node { name } } }""",
)
assert res["testResults"] == {"edges": [{"node": {"name": test2.name}}]}

def test_flaky_filter_on_test_results(self) -> None:
repo = RepositoryFactory(author=self.owner, active=True, private=True)
test = TestFactory(repository=repo)
Expand Down Expand Up @@ -167,6 +194,7 @@ def test_failed_filter_on_test_results(self) -> None:
assert res["testResults"] == {"edges": [{"node": {"name": test2.name}}]}

def test_skipped_filter_on_test_results(self) -> None:
# note - this test guards against division by zero errors for the failure/flake rate
repo = RepositoryFactory(author=self.owner, active=True, private=True)
test = TestFactory(repository=repo)
test2 = TestFactory(repository=repo)
Expand Down Expand Up @@ -367,8 +395,8 @@ def test_last_duration_ordering_on_test_results(self) -> None:
)
assert res["testResults"] == {
"edges": [
{"node": {"name": test.name, "lastDuration": 0.0}},
{"node": {"name": test_2.name, "lastDuration": 0.0}},
{"node": {"name": test.name, "lastDuration": 2.0}},
{"node": {"name": test_2.name, "lastDuration": 3.0}},
]
}

Expand Down Expand Up @@ -402,8 +430,8 @@ def test_desc_last_duration_ordering_on_test_results(self) -> None:
)
assert res["testResults"] == {
"edges": [
{"node": {"name": test_2.name, "lastDuration": 0.0}},
{"node": {"name": test.name, "lastDuration": 0.0}},
{"node": {"name": test_2.name, "lastDuration": 3.0}},
{"node": {"name": test.name, "lastDuration": 2.0}},
]
}

Expand Down Expand Up @@ -548,6 +576,142 @@ def test_desc_failure_rate_ordering_on_test_results(self) -> None:
]
}

def test_desc_failure_rate_ordering_on_test_results_with_after(self) -> None:
repo = RepositoryFactory(author=self.owner, active=True, private=True)
test = TestFactory(repository=repo)
_ = DailyTestRollupFactory(
test=test,
date=datetime.date.today() - datetime.timedelta(days=1),
repoid=repo.repoid,
pass_count=1,
fail_count=1,
)
_ = DailyTestRollupFactory(
test=test,
date=datetime.date.today(),
repoid=repo.repoid,
pass_count=3,
fail_count=0,
)
test_2 = TestFactory(repository=repo)
_ = DailyTestRollupFactory(
test=test_2,
date=datetime.date.today(),
repoid=repo.repoid,
pass_count=2,
fail_count=3,
)
res = self.fetch_test_analytics(
repo.name,
"""testResults(ordering: { parameter: FAILURE_RATE, direction: DESC }, first: 1) { edges { node { name failureRate } }, pageInfo { hasNextPage, hasPreviousPage, startCursor, endCursor }, totalCount }""",
)

assert res["testResults"] == {
"edges": [
{"node": {"name": test_2.name, "failureRate": 0.6}},
],
"pageInfo": {
"endCursor": base64_encode_string(f"0.6|{test_2.name}"),
"hasNextPage": True,
"hasPreviousPage": False,
"startCursor": base64_encode_string(f"0.6|{test_2.name}"),
},
"totalCount": 2,
}

res = self.fetch_test_analytics(
repo.name,
"""testResults(ordering: { parameter: FAILURE_RATE, direction: DESC }, first: 1, after: "%s") { edges { node { name failureRate } }, pageInfo { hasNextPage, hasPreviousPage, startCursor, endCursor }, totalCount }"""
% res["testResults"]["pageInfo"]["endCursor"],
)

assert res["testResults"] == {
"edges": [
{"node": {"name": test.name, "failureRate": 0.2}},
],
"pageInfo": {
"endCursor": base64_encode_string(f"0.2|{test.name}"),
"hasNextPage": False,
"hasPreviousPage": False,
"startCursor": base64_encode_string(f"0.2|{test.name}"),
},
"totalCount": 2,
}

res = self.fetch_test_analytics(
repo.name,
"""testResults(ordering: { parameter: FAILURE_RATE, direction: ASC }, first: 1) { edges { node { name failureRate } }, pageInfo { hasNextPage, hasPreviousPage, startCursor, endCursor }, totalCount }""",
)

assert res["testResults"] == {
"edges": [
{"node": {"name": test.name, "failureRate": 0.2}},
],
"pageInfo": {
"endCursor": base64_encode_string(f"0.2|{test.name}"),
"hasNextPage": True,
"hasPreviousPage": False,
"startCursor": base64_encode_string(f"0.2|{test.name}"),
},
"totalCount": 2,
}

res = self.fetch_test_analytics(
repo.name,
"""testResults(ordering: { parameter: FAILURE_RATE, direction: ASC }, first: 1, after: "%s") { edges { node { name failureRate } }, pageInfo { hasNextPage, hasPreviousPage, startCursor, endCursor }, totalCount }"""
% res["testResults"]["pageInfo"]["endCursor"],
)

assert res["testResults"] == {
"edges": [
{"node": {"name": test_2.name, "failureRate": 0.6}},
],
"pageInfo": {
"endCursor": base64_encode_string(f"0.6|{test_2.name}"),
"hasNextPage": False,
"hasPreviousPage": False,
"startCursor": base64_encode_string(f"0.6|{test_2.name}"),
},
"totalCount": 2,
}

res = self.fetch_test_analytics(
repo.name,
"""testResults(ordering: { parameter: FAILURE_RATE, direction: ASC }, last: 2) { edges { node { name failureRate } }, pageInfo { hasNextPage, hasPreviousPage, startCursor, endCursor }, totalCount }""",
)

assert res["testResults"] == {
"edges": [
{"node": {"name": test_2.name, "failureRate": 0.6}},
{"node": {"name": test.name, "failureRate": 0.2}},
],
"pageInfo": {
"endCursor": base64_encode_string(f"0.2|{test.name}"),
"hasNextPage": False,
"hasPreviousPage": False,
"startCursor": base64_encode_string(f"0.6|{test_2.name}"),
},
"totalCount": 2,
}

res = self.fetch_test_analytics(
repo.name,
"""testResults(ordering: { parameter: FAILURE_RATE, direction: ASC }, last: 1) { edges { node { name failureRate } }, pageInfo { hasNextPage, hasPreviousPage, startCursor, endCursor }, totalCount }""",
)

assert res["testResults"] == {
"edges": [
{"node": {"name": test_2.name, "failureRate": 0.6}},
],
"pageInfo": {
"endCursor": base64_encode_string(f"0.6|{test_2.name}"),
"hasNextPage": False,
"hasPreviousPage": True,
"startCursor": base64_encode_string(f"0.6|{test_2.name}"),
},
"totalCount": 2,
}

def test_flake_rate_filtering_on_test_results(self) -> None:
repo = RepositoryFactory(author=self.owner, active=True, private=True)
test = TestFactory(repository=repo)
Expand Down Expand Up @@ -753,6 +917,43 @@ def test_test_results_aggregates_no_history(self) -> None:
"totalSlowTestsPercentChange": None,
}

def test_test_results_aggregates_no_history_7_days(self) -> None:
repo = RepositoryFactory(
author=self.owner, active=True, private=True, branch="main"
)

for i in range(0, 7):
test = TestFactory(repository=repo)
_ = DailyTestRollupFactory(
test=test,
repoid=repo.repoid,
branch="main",
fail_count=1 if i % 3 == 0 else 0,
skip_count=1 if i % 6 == 0 else 0,
pass_count=1,
avg_duration_seconds=float(i),
last_duration_seconds=float(i),
date=datetime.date.today() - datetime.timedelta(days=i),
)

res = self.fetch_test_analytics(
repo.name,
"""testResultsAggregates(interval: INTERVAL_7_DAY) { totalDuration, slowestTestsDuration, totalFails, totalSkips, totalSlowTests, totalDurationPercentChange, slowestTestsDurationPercentChange, totalFailsPercentChange, totalSkipsPercentChange, totalSlowTestsPercentChange }""",
)

assert res["testResultsAggregates"] == {
"totalDuration": 30.0,
"totalDurationPercentChange": None,
"slowestTestsDuration": 12.0,
"slowestTestsDurationPercentChange": None,
"totalFails": 3,
"totalFailsPercentChange": None,
"totalSkips": 2,
"totalSkipsPercentChange": None,
"totalSlowTests": 1,
"totalSlowTestsPercentChange": None,
}

def test_flake_aggregates(self) -> None:
repo = RepositoryFactory(
author=self.owner, active=True, private=True, branch="main"
Expand Down Expand Up @@ -924,7 +1125,7 @@ def test_flake_aggregates_7_days(self) -> None:

res = self.fetch_test_analytics(
repo.name,
"""flakeAggregates(history: INTERVAL_7_DAY) { flakeCount, flakeRate, flakeCountPercentChange, flakeRatePercentChange }""",
"""flakeAggregates(interval: INTERVAL_7_DAY) { flakeCount, flakeRate, flakeCountPercentChange, flakeRatePercentChange }""",
)

assert res["flakeAggregates"] == {
Expand Down
2 changes: 1 addition & 1 deletion graphql_api/tests/test_test_result.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def test_fetch_test_result_last_duration(self) -> None:
result["owner"]["repository"]["testAnalytics"]["testResults"]["edges"][0][
"node"
]["lastDuration"]
== 0.0
Copy link
Contributor

@ajay-sentry ajay-sentry Oct 17, 2024

Choose a reason for hiding this comment

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

do you know why this guy changed?

Copy link
Contributor Author

@joseph-sentry joseph-sentry Oct 18, 2024

Choose a reason for hiding this comment

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

we stopped automatically setting the lastDuration value to 0 from when we stopped computing it because we thought it was singlehandedly ruining the perf of the query

Copy link
Contributor

@ajay-sentry ajay-sentry Oct 18, 2024

Choose a reason for hiding this comment

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

ahhhh okay yeah i remember

== 1.0
)

def test_fetch_test_result_avg_duration(self) -> None:
Expand Down
2 changes: 1 addition & 1 deletion graphql_api/types/inputs/test_results_filters.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ input TestResultsFilters {
parameter: TestResultsFilterParameter
test_suites: [String!]
flags: [String!]
history: MeasurementInterval
interval: MeasurementInterval
term: String
}

Expand Down
4 changes: 2 additions & 2 deletions graphql_api/types/test_analytics/test_analytics.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ type TestAnalytics {
): TestResultConnection! @cost(complexity: 10, multipliers: ["first", "last"])

"Test results aggregates are analytics data totals across all tests"
testResultsAggregates(history: MeasurementInterval): TestResultsAggregates
testResultsAggregates(interval: MeasurementInterval): TestResultsAggregates

"Flake aggregates are flake totals across all tests"
flakeAggregates(history: MeasurementInterval): FlakeAggregates
flakeAggregates(interval: MeasurementInterval): FlakeAggregates
}

type TestResultConnection {
Expand Down
Loading
Loading