Skip to content

Don't display invalid results#406

Merged
camillobruni merged 9 commits into
WebKit:mainfrom
camillobruni:2024-04-08_more_robust_results
Jul 25, 2024
Merged

Don't display invalid results#406
camillobruni merged 9 commits into
WebKit:mainfrom
camillobruni:2024-04-08_more_robust_results

Conversation

@camillobruni
Copy link
Copy Markdown
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

@camillobruni camillobruni requested a review from julienw April 8, 2024 13:12
@flashdesignory
Copy link
Copy Markdown
Contributor

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

@camillobruni
Copy link
Copy Markdown
Contributor Author

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

@rniwa
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Screenshot 2024-04-09 at 11 04 00

@camillobruni
Copy link
Copy Markdown
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
Copy Markdown
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!

Comment thread resources/benchmark-runner.mjs Outdated
Comment thread resources/benchmark-runner.mjs Outdated
@camillobruni
Copy link
Copy Markdown
Contributor Author

@rniwa could you have another look maybe?

@camillobruni
Copy link
Copy Markdown
Contributor Author

@rniwa do you have additional feedback or would be ok with landing with your proposed changes?

@camillobruni camillobruni merged commit a999ad1 into WebKit:main Jul 25, 2024
camillobruni added a commit to camillobruni/Speedometer that referenced this pull request Jan 29, 2025
This partially fixes WebKit#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
camillobruni added a commit that referenced this pull request Feb 18, 2025
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
- Merge of original PR  #406
camillobruni added a commit to camillobruni/Speedometer that referenced this pull request Jun 2, 2026
b475f2d Perf-Dashboard.Render sync phase sets location.hash, but the actual DOM (WebKit#488) (WebKit#496)
b270564 (origin/3.1, 3.1) Release/3.1 428 - add id to input (NEW - with correct base to merge into) (WebKit#494)
aba1d5e (thor/release/3.1, flashd/release/3.1) Deconfuse Geomean Metric and geomean aggregate value (WebKit#408) (WebKit#480)
ce03cfb Don't display invalid results (WebKit#406) (WebKit#479)
6f25345 Update Version Text from 3.0 to 3.1 (WebKit#485)
cc9ee08  Release/3.1 - merge 400 (rename className -> class) (WebKit#483)
e12e84c Release/3.1 - merge 448 (news site article ids nuxt) (WebKit#482)
8a4a566 Release/3.1 - merge 447 (news site article ids next) (WebKit#481)

Change-Id: If0b19391128b82389eda1a61d33dc4e2be2cd180
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