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

Don't display invalid results #406

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

camillobruni
Copy link
Contributor

@camillobruni camillobruni commented Apr 8, 2024

This partially fixes #399 by not display non-finite scores which are caused by 0-measured suite results.

  • For non-positive or non-finite scores, the following summary page is displayed. Note that the detailed page is still accessible since you might find some useful information there.
  • Additionally a console.error is generated for each zero-sum suite
Screenshot 2024-04-08 at 11 17 26

@flashdesignory
Copy link
Contributor

should we display a message to the user, explaining what happened and that the details view might have useful information?

@camillobruni
Copy link
Contributor Author

Updated version with a better description:
Screenshot 2024-04-08 at 16 57 52

@rniwa
Copy link
Member

rniwa commented Apr 8, 2024

Rather than "Invalid Score", how about "Error"? Also, we should probably stop right away when we detect an invalid value instead of running 'til completion.

@camillobruni
Copy link
Contributor Author

Screenshot 2024-04-09 at 11 04 00

@camillobruni
Copy link
Contributor Author

Updated the code to stop after the first iteration with a broken score (tracking errors across the TestInvoker seems a bit more tricky at the moment).

Copy link
Contributor

@julienw julienw left a comment

Choose a reason for hiding this comment

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

I don't have a strong opinion about the exact behavior but this looks reasonable to me. Thanks!

@camillobruni
Copy link
Contributor Author

@rniwa could you have another look maybe?

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.

With privacy.resistFingerprinting set to true in Firefox, Speedometer shows the result Infinity
4 participants