-
Notifications
You must be signed in to change notification settings - Fork 17
Implement end2end tests #11
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
Implement end2end tests #11
Conversation
| return value | ||
| } | ||
|
|
||
| if (typeof(URLSearchParams) !== "undefined") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it will always set startDelay to undefined when the there's no startDelay URL parameter. I think that's fine for testIterationCount and testWorstCaseCount since they'd always be undefined on the web as it is. Although maybe it's worth changing there too so folks aren't surprised?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hoped to make the initialization explicit by the lines above globalThis.startDelay ??= undefined; at least for me that's a bit more consistent across all configuration params.
Medium term I'd like to adapt the Params object from Speedometer (this also allows us to easily copy over the ?developerMode UI).
Maybe I missed your point here, but when running via the shell we now also always initialize startDelay=undefined.
With the html runner we'd initialize to undefined by default (and set it again to undefined if there is no matching url param)
But generally not feeling strongly about this one :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the issue is when you do index.html?report=true startDelay ends up as undefined rather than 4000. If you moved the report check below this and changed it to if (shouldReport) globalThis.startDelay ??= 4000 I think that would fix it.
Safari, at least, relies on the startDelay to make sure the browser has calmed down after launching enough to prevent noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, then I understood correctly.
Keeping the old behavior with report=true sg.
|
I've now merged in the latest PR from speedomter to use the local-web-server package so we don't have to write our own little webserver. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
npm test:chrome,npm test:firefox,npm test:safariworstCaseCountanditerationCountto lower test durationiterationstoBenchmark.validateto fix unipoker expectationsJetStreamReadyandJetStreamDoneevents to theDriverfor easier testing