Skip to content

"#4751 - fix for incorrect fail and pass count in summary report" #4752

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

Closed
wants to merge 2 commits into from

Conversation

vinodrkumars
Copy link

What?

Fix for incorrect fail and pass count in summary report

Why?

Bug#4751
Incorrect fail and pass count in summary report

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the release notes: link
  • I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
  • I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

@vinodrkumars vinodrkumars requested a review from a team as a code owner April 30, 2025 00:52
@vinodrkumars vinodrkumars requested review from inancgumus and the-it and removed request for a team April 30, 2025 00:52
@CLAassistant
Copy link

CLAassistant commented Apr 30, 2025

CLA assistant check
All committers have signed the CLA.

@oleiade
Copy link
Member

oleiade commented Apr 30, 2025

Replied in issue #4751, and asked for clarifications 🙇🏻

In the meantime, I will mark this PR as blocked, as I don't think the code it modifies is strictly related to the reported issue. The issue is about wrong numbers reported regarding the browser_http_req_failed metric, but this PR modifies the code that specifically handles checks results instead.

cc @joanlopez

@andrewslotin andrewslotin requested review from oleiade and removed request for the-it May 14, 2025 12:42
Comment on lines -297 to +298
summaryGroup.Checks.Metrics.Fail.Values["passes"] = totalChecks - successChecks
summaryGroup.Checks.Metrics.Fail.Values["fails"] = successChecks
summaryGroup.Checks.Metrics.Fail.Values["passes"] = successChecks
summaryGroup.Checks.Metrics.Fail.Values["fails"] = totalChecks - successChecks
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, note that these lines:

  • First and foremost, refer to checks metrics, not browser metrics, as reported in the issue Incorrect http request pass and fail count in summary report file. #4751
  • Second, and despite I know it can be confusing, "passes" and "fails" there are the identifiers used for values of *metrics.RateSink, and as you can see, it refers to: summaryGroup.Checks.Metrics.Fail. So, "passes" really correspond to: "how many times checks failed" (note the difference with summaryGroup.Checks.Metrics.Success.

@joanlopez
Copy link
Contributor

joanlopez commented May 15, 2025

Closing, because as I mentioned in #4752 (comment), it looks like a confusion, rather than a real bug.


Thanks anyway @vinodrkumars for your willingness to contribute, any other contribution will be more than welcome! 🙇🏻

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.

5 participants