fix: vite-dev-server windows #25889
fix: vite-dev-server windows #25889
Conversation
6 flaky tests on run #44300 ↗︎
Details:
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Test | ||
|---|---|---|
| scaffolding component testing > vuecli4vue2 > scaffolds component testing for Vue CLI 4 w/ Vue 2 project |
Screenshot
|
|
cypress\e2e\create-from-component.cy.ts • 1 flaky test • app-e2e
| Test | ||
|---|---|---|
| ... > Shows create from component card for React projects |
Screenshot
|
|
cypress\e2e\specs.cy.ts • 2 flaky tests • app-e2e
| Test | ||
|---|---|---|
| ... > create template spec > shows dialog that can be dismissed with Close (x) button press |
Screenshot
|
|
| ... > opens config file in ide from SpecPattern |
Screenshot
|
|
cypress\e2e\specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e
| Test | ||
|---|---|---|
| App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs |
Screenshot
|
|
cypress\e2e\cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e
| Test | ||
|---|---|---|
| Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution |
Screenshot
|
|
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
| let testFileAbsolutePathRoute | ||
|
|
||
| if (CypressInstance.config('platform') === 'win32') { | ||
| testFileAbsolutePathRoute = `${devServerPublicPathRoute}/@fs/${CypressInstance.spec.absolute}` | ||
| } else { | ||
| testFileAbsolutePathRoute = `${devServerPublicPathRoute}/@fs${CypressInstance.spec.absolute}` | ||
| } |
There was a problem hiding this comment.
I was going to suggest a adding a comment here. The difference between the 2 lines is one character in the middle of a string, which is hard to notice at glance.
But we could also change the code so that the difference is more obvious? Something like:
| let testFileAbsolutePathRoute | |
| if (CypressInstance.config('platform') === 'win32') { | |
| testFileAbsolutePathRoute = `${devServerPublicPathRoute}/@fs/${CypressInstance.spec.absolute}` | |
| } else { | |
| testFileAbsolutePathRoute = `${devServerPublicPathRoute}/@fs${CypressInstance.spec.absolute}` | |
| } | |
| let testFileAbsolutePathRoute = `${devServerPublicPathRoute}/@fs` | |
| if (CypressInstance.config('platform') === 'win32') { | |
| testFileAbsolutePathRoute += `/${CypressInstance.spec.absolute}` | |
| } else { | |
| testFileAbsolutePathRoute += `${CypressInstance.spec.absolute}` | |
| } |
There was a problem hiding this comment.
Might be clearer to remove the OS check and just normalize the absolute path to ensure it doesn't have a leading slash before we work with it
// Normalize path to not include a leading slash (different on Win32 vs Unix)
const normalizedAbsolutePath = CypressInstance.spec.absolute.replace(/^\//, '')
const testFileAbsolutePathRoute = `${devServerPublicPathRoute}/@fs/${normalizedAbsolutePath}`
There was a problem hiding this comment.
I guess - my concern here is actually the fact we need a comment and a regular expression seems much more complex, or the string concatenation, both seem more complex than just a good old if/else statement, where you can immediately glance and see the difference.
There was a problem hiding this comment.
I agree we should add a comment here explaining why this is required, let me update that
There was a problem hiding this comment.
Okay, I thought more - let's go with normalzing, it does look better, after all - thanks!
|
Windows passing, going to merge. |
Fixing a regression introduced by #25801 (in develop, but not yet in the wild) which broke
vite-dev-serveron Windows.Testing: Observe windows CI, it's passing - as opposed to develop, where many Vite Dev Server tests are failing.