-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KAFKA-18223 Flaky test report script #17938
Conversation
…ded support for reporting types flaky test regression and clear tests from quarantine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santhoshct thanks for working on this! This is an excellent start 👍
The report is very detailed, which is great, but can we also include a summary at the top? For example, in the report I just ran it would be great to see something like this for the worst flaky tests:
org.apache.kafka.message.checker.MetadataSchemaCheckerToolTest
→ testVerifyEvolutionGit() 83.33%
org.apache.kafka.tiered.storage.integration.TransactionsWithTieredStoreTest
→ testReadCommittedConsumerShouldNotSeeUndecidedData(String, String)[2] 46.17%
→ testBumpTransactionalEpochWithTV2Enabled(String, String, boolean)[1] ...
→ testBumpTransactionalEpochWithTV2Enabled(String, String, boolean)[2] ...
→ testBumpTransactionalEpochWithTV2Disabled(String, String, boolean)[1] ...
→ testBumpTransactionalEpochWithTV2Disabled(String, String, boolean)[2] ...
kafka.api.TransactionsTest
→ testBumpTransactionalEpochWithTV2Enabled(String, String, boolean)[1] 26.09%
→ testBumpTransactionalEpochWithTV2Enabled(String, String, boolean)[2] ...
→ testBumpTransactionalEpochWithTV2Disabled(String, String, boolean)[1] ...
@@ -0,0 +1,863 @@ | |||
import os |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need a license here. See other scripts for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the license
.github/scripts/requirements.txt
Outdated
@@ -1,3 +1,4 @@ | |||
<<<<<<< HEAD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like leftovers from a merge conflict
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed.
""" | ||
return f'project:{project} buildStartTime:[{chunk_start.isoformat()} TO {chunk_end.isoformat()}] gradle.requestedTasks:{test_type}' | ||
|
||
def process_chunk(self, chunk_start: datetime, chunk_end: datetime, project: str, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this and other methods with many arguments, use the following PEP-8 style:
def process_chunk(
self,
chunk_start: datetime,
chunk_end: datetime,
project: str,
test_type: str,
remaining_build_ids: set,
max_builds_per_request: int) -> Dict[str, BuildInfo]:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected this.
reverse=True | ||
) | ||
|
||
print(f"\nFound {len(sorted_tests)} high-priority quarantined test containers:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The count here should be the number of flaky test cases rather than flaky test classes (containers).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we should not use the term "container" in the report since it's kind of confusion. As far as I know, for our purposes a container is always a test class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected this to test class.
|
||
# Show test case timeline | ||
if test_case.timeline: | ||
print("\n Recent Executions:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For "Recent" things, let's indicate how far back we're showing in the output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added info about the runs.
2. Corrected the method signature to pep 8 style. 3. Added license file to the script 4. Corrected the develocity specific term "container" to more generic test classes. 5. Added more info to the recent executions to make it more descriptive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santhoshct I've run it locally and the output looks great! I'm going to go ahead and merge this so we can let people start trying it out.
Adds a python script to generate a detailed flaky test report using the Develocity API Reviewers: David Arthur <[email protected]>
Summary
This pull request introduces a new script, develocity_reports.py, designed to enhance our detailed reports on flaky tests. It leverages the Develocity API to fetch and analyze test results, focusing on identifying and reporting quarantined tests with high failure rates. The script is intended to help developers/CI quickly identify problematic tests that require attention, thereby improving the overall quality of our codebase.
Changes
###Updates
Testing
Test Analysis Report (2024-12-03 08:23:37 UTC).txt
Updated report with Test Report summary section:
Committer Checklist (excluded from commit message)