Skip to content
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

Fix issue where a test incorrectly reported as "Passed on Retry" #252

Merged
merged 3 commits into from
Jan 16, 2025

Conversation

nprizal
Copy link
Contributor

@nprizal nprizal commented Jan 15, 2025

Description

We found a bug where a test marked as "Passed on retry". This is happening when there are different tests with the same scope and name. Currently, bktec will use scope and name to identify a test. If there are 2 tests with the same scope and name it will treat those tests as the same test with 2 execution counts, leading to incorrect reporting. The initial implementation was made to follow the server-side implementation so that bktec can match a local test with the list of muted tests received from the server.

This PR resolves the issues by introducing 2 different ways of identifying a test. When reading the result from test runner, bktec will identify a test using scope, name and path to make sure tests with the same name and scope counted as separate test. However, when checking if a test is muted, it identifies the test using scope and name only to follow the server-side implementation.

This PR will resolve #236

@nprizal nprizal requested a review from a team as a code owner January 15, 2025 22:14
@nprizal nprizal changed the title Update testIdentifier function to include test path Fix incorrect report where a test marked as "Passed on Retry" Jan 15, 2025
@nprizal nprizal changed the title Fix incorrect report where a test marked as "Passed on Retry" Fix issue where a test incorrectly reported as "Passed on Retry" Jan 15, 2025
@nprizal nprizal marked this pull request as draft January 15, 2025 22:55
@nprizal nprizal force-pushed the tet-491-passed-on-retry-count-incorrect branch from c566cc7 to c8f1647 Compare January 15, 2025 23:21
@nprizal nprizal marked this pull request as ready for review January 15, 2025 23:36
func testIdentifier(testCase plan.TestCase) string {
return testCase.Scope + "/" + testCase.Name + "/" + testCase.Path
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is path information available for all supported frameworks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, except for Cypress, as we don't currently parse/read Cypress result because Cypress can't output result into file out of the box. We already read test path for RSpec and Playwright, so no changes are required. As for Jest, I updated the runner to read the test path in this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, mute and retry aren't even supported in Cypress so that is fine :)

@nprizal nprizal merged commit 16d2577 into main Jan 16, 2025
1 check passed
@nprizal nprizal deleted the tet-491-passed-on-retry-count-incorrect branch January 16, 2025 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Question] "On Retry" is > 0. But where are we retrying those tests?
2 participants