-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(browser): use client-side timer for browser mode duration #8699
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for vitest-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Looks like we need to initialize the configuration correctly... |
# Conflicts: # packages/ui/client/composables/explorer/tree.ts
| // - Browser iframe creation and orchestration | ||
| // - Broadcast channel communication between the UI and the test iframe. | ||
| // The terminal reporter, in contrast, only aggregates the pure test execution | ||
| // time reported by the browser orchestrator. |
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 don't see the point of this, we want to show the same duration as the terminal, not the time when you click on run
There are events like onTestRunStart/onTestRunEnd that should be used to calculate the different
I am also not sure why we need to separate browser duration and UI duration, it shouldn't matter
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.
will check those hooks
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.
will add a new hook (onStarted) and send the executionTime from the websocket reporter, maybe we can send the full data shown in the default reporter, but will require some more work since the websocket just implements the Reporter interface => change the logic at client to display that executionTime instead using actual logic for the total time (new PR):
Description
This PR measures the wall-clock time from the UI's perspective (from test run trigger to "finished" event), which is the most accurate measure of the perceived developer experience. It will naturally be higher than the duration reported in the terminal, as it includes the entire browser test orchestration:
The terminal reporter, in contrast, only aggregates the pure test execution time reported by the browser orchestrator.
/cc @JessicaSachs
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets
feat:,fix:,perf:,docs:, orchore:.