Fix thresholds evaluated incorrectly

[Fernando-hub527]

What?

This PR ensures that once thresholds are defined, they are still evaluated correctly even if check(...) is never called.

Why?

Guarantee that if an error occurs before any checks are executed, the threshold evaluation will correctly register a failure.

Checklist

• [x] I have performed a self-review of my code.
• [x] I have commented on my code, particularly in hard-to-understand areas.
• [x] 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)

Closes #4283

[Fernando-hub527]

As far as I could determine, the issue was happening because, to avoid a division by zero, the rate metric wasn’t populated when the total number of checks was zero (see code here). That means if an error occurred before any checks were executed, the threshold failure was never triggered.

Here’s an example of the threshold evaluation after the fix:

code:

result:

[joanlopez]

Hi @Fernando-hub527,

We're discussing internally if really want to merge this new behavior for thresholds as-is, or perhaps offer another workaround to sort it out.

So, could you please split it into two different PRs, one for changing the threshold evaluation behavior and the other one to fix the NaN%? You can leave this one as-is, just extract the small bits into another one.

This way we can merge this part of your contribution and close https://github.com/grafana/k6/issues/4842. And later continue with the ongoing discussion around thresholds. cc/ @mstoykov for visibility.

Thanks! 🙇🏻 🙌🏻

[Fernando-hub527]

Hi @joanlopez, sorry for the delay. Here's the PR containing only the fix for the NaN issue: #4878. If you think it would be useful to share the discussion, I'd be happy to help.

[joanlopez]

Hey @Fernando-hub527,

I'm going to close this pull request because after some internal discussions, I even stronger believe we won't want to change this behavior on thresholds, as it would introduce a breaking change, and it's unlikely that's always going to be the desired behavior by users, while it won't be configurable.

So, we'll likely agree on another workaround, like "exposing" the "total" value for Rate metrics, letting users to define another threshold that ensures "total > 0"; or adding a new option, like abortOnFail, but specifically for the case where there's no data at all, like failWhenNoData.

Anyway, thanks for the other half of the PR that you split and that has already been merged, as that was really a small bug to be fixed. Really appreciated! 🙌🏻