chore: fix tests that fail on windows (#21055)

* check if failing

* update timeouts

* move cy code inside it

* revert code

* fixing test

* update timeout

* update tests

* fix: handle race condition with windows and the cri client

* fix: handle race condition with windows and the cri client

* fix: handle race condition with windows and the cri client

* Refactor and unit tests fixes

* Further refactoring and error handling cleanup

* update test timeout

* Update timeout for spec subscription e2e test

* Fix issue with ssh keys in circle

* Apply suggestions from code review

Co-authored-by: Zach Bloomquist <git@chary.us>

* update circle.yml

* Apply suggestions from code review

Co-authored-by: Zach Bloomquist <git@chary.us>

* add comment and fix lint

* add guide for platform differences

* timeout

* Update circle.yml

* Update guides/remaining-platform-agnostic.md

Co-authored-by: Zach Bloomquist <git@chary.us>

* update docs and comments

* rename file

* Apply suggestions from code review

Co-authored-by: Ryan Manuel <ryanm@cypress.io>

Co-authored-by: Ryan Manuel <ryanm@cypress.io>
Co-authored-by: Zach Bloomquist <git@chary.us>
Co-authored-by: Tyler Biethman <tbiethman@users.noreply.github.com>

circle.yml

@@ -29,7 +29,7 @@ mainBuildFilters: &mainBuildFilters
       only:
         - develop
         - 10.0-release
-        - unify-1449-check-path-length-in-build
+        - lmiller1990/windows-fails
 
 # uncomment & add to the branch conditions below to disable the main linux
 # flow if we don't want to test it for a certain branch
@@ -47,7 +47,7 @@ macWorkflowFilters: &mac-workflow-filters
     or:
     - equal: [ develop, << pipeline.git.branch >> ]
     - equal: [ '10.0-release', << pipeline.git.branch >> ]
-    - equal: [ unify-1449-check-path-length-in-build, << pipeline.git.branch >> ]
+    - equal: [ lmiller1990/windows-fails, << pipeline.git.branch >> ]
     - matches:
           pattern: "-release$"
           value: << pipeline.git.branch >>
@@ -57,7 +57,7 @@ windowsWorkflowFilters: &windows-workflow-filters
     or:
     - equal: [ develop, << pipeline.git.branch >> ]
     - equal: [ '10.0-release', << pipeline.git.branch >> ]
-    - equal: [ 'tgriesser/chore/fix-windows-build', << pipeline.git.branch >> ]
+    - equal: [ lmiller1990/windows-fails, << pipeline.git.branch >> ]
     - matches:
           pattern: "-release$"
           value: << pipeline.git.branch >>
@@ -1714,7 +1714,7 @@ jobs:
       - run:
           name: Check current branch to persist artifacts
           command: |
-            if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "unify-1449-check-path-length-in-build" && "$CIRCLE_BRANCH" != "10.0-release" ]]; then
+            if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "lmiller1990/windows-fails" && "$CIRCLE_BRANCH" != "10.0-release" ]]; then
               echo "Not uploading artifacts or posting install comment for this branch."
               circleci-agent step halt
             fi

guides/README.md

@@ -13,6 +13,7 @@ For general contributor information, check out [`CONTRIBUTING.md`](../CONTRIBUTI
 * [Building release artifacts](./building-release-artifacts.md)
 * [Code signing](./code-signing.md)
 * [Determining the next version of Cypress to be released](./next-version.md)
+* [Remaining Platform Agnostic](./remaining-platform-agnostic.md)
 * [Error handling](./error-handling.md)
 * [Patching packages](./patch-package.md)
 * [Release process](./release-process.md)

guides/writing-cross-platform-javascript.md

@@ -0,0 +1,85 @@
+# Writing Cross-Platform JavaScript
+
+Cypress works on Linux, macOS and Windows. This includes both installing from npm, as well as for local development. Code should be written in a platform agnostic style.
+
+## Handling File Paths
+
+Throughout the code base, we access the file system in various ways, and need to be conscious of how we do so to ensure Cypress can be used and developed seamlessly on multiple platforms. One thing to keep in mind is file paths and file separators. macOS and Linux systems use `/`, and Windows uses `\`.
+
+
+As a general rule, we want to use **native paths** where possible. There are a few reasons for this. Whereever we display a file path, we want to use the native file separator, since that is what the user will expect on their platform. In general, we can use the Node.js `path` module to handle this:
+
+```js
+// on linux-like systems
+path.join('cypress', 'e2e') //=> `cypress/e2e`
+
+// on Windows
+path.join('cypress', 'e2e') //=> `cypress\e2e`
+```
+
+There are some exceptions to this, namely the [`globby`](https://www.npmjs.com/package/globby) module, which only supports `/` (see [here](https://github.com/sindresorhus/globby#api)) when writing glob patterns. In these cases, where an API is posix only, you can use `path.posix.join`, which will always use `/`, even on a Windows system:
+
+```js
+// don't do 
+const files = await globby('my-project\cypress\e2e\**\*')
+
+// do
+const files = await globby('my-project/cypress/e2e/**/*')
+
+// or you can convert it by splitting by path.sep
+// and joining with `path.posix.join`
+const glob = path.posix.join('my-project/cypress/e2e/**/*'.split(path.sep))
+```
+
+The general rule of using `path` where possible applies to moving around the file system, too:
+
+```js
+path.resolve('../', '/../', '../')
+// '/home' on Linux
+// '/Users' on OSX
+// 'C:\\Users' on Windows
+```
+
+In general, you want to avoid writing file system code using `/` and `\`, and use Node.js APIs where possible - those are cross platform and guarenteed to work.
+
+## Use Node.js Scripts 
+
+For many developers, it's tempting to write a quick bash script to automate tasks. Maybe you'd like to delete all `.js` files, so you add a script to `package.json`:
+
+```json
+{ 
+  "scripts": {
+    "clean": "rm -rf **/*.js"
+  }
+}
+```
+
+This will stop developers on Windows from running `yarn clean` unless they are specifically using a POSIX shell (like Git Bash). Instead, opt for a Node.js script where possible, or use a cross-platform Node.js module. In this case, we could use the [`rimraf`](https://www.npmjs.com/package/rimraf) module:
+
+```json
+{ 
+  "devDependencies": {
+    "rimraf": "3.0.2",
+  },
+  "scripts": {
+    "clean": "rimraf '**/*.js'"
+  }
+}
+```
+
+Now your script is cross-platform.
+
+## Use the os Module
+
+You can use the `os` module to handle platform differences. One such example is line endings; `\n` on linux systems, and `\r\n` on Windows. Instead. use `os.EOL`. To check the current platform, use `os.arch()`:
+
+```ts
+import os from 'os'
+
+os.EOL // \n on linux, \r\n on windows
+
+os.platform()
+// 'linux' on Linux
+// 'win32' on Windows (32-bit / 64-bit)
+// 'darwin' on OSX
+```

packages/app/cypress/e2e/runner/retries.mochaEvents.cy.ts

@@ -2,7 +2,13 @@ import { runSpec } from './support/spec-loader'
 import { runCypressInCypressMochaEventsTest } from './support/mochaEventsUtils'
 import { snapshots } from './retries.mochaEvents.snapshots'
 
-describe('src/cypress/runner retries mochaEvents', { retries: 0 }, () => {
+/**
+ * These tests are slow, particular slow on windows, thus the
+ * 7500m timeout.
+ * TODO: Find out if they are objectively slower on windows than on linux,
+ * and if it's a 10.x specific performance regression.
+ */
+describe('src/cypress/runner retries mochaEvents', { retries: 0, defaultCommandTimeout: 7500 }, () => {
   // NOTE: for test-retries
 
   it('simple retry', (done) => {

packages/app/cypress/e2e/runner/runner.mochaEvents.cy.ts

@@ -2,7 +2,7 @@ import { runSpec } from './support/spec-loader'
 import { runCypressInCypressMochaEventsTest } from './support/mochaEventsUtils'
 import { snapshots } from './runner.mochaEvents.snapshots'
 
-describe('src/cypress/runner', { retries: 0 }, () => {
+describe('src/cypress/runner', { retries: 0, defaultCommandTimeout: 7500 }, () => {
   describe('tests finish with correct state', () => {
     describe('hook failures', () => {
       it('fail in [before]', (done) => {

packages/app/cypress/e2e/runs.cy.ts

@@ -255,7 +255,7 @@ describe('App: Runs', { viewportWidth: 1200 }, () => {
       cy.get('[data-cy="selectProject"] button').click()
       cy.findByText('Mock Project').click()
       cy.findByText(defaultMessages.runs.connect.modal.selectProject.connectProject).click()
-      cy.get('[data-cy="runs"]')
+      cy.get('[data-cy="runs"]', { timeout: 7500 })
     })
   })
 

packages/app/cypress/e2e/specs_list_actual_git_repo.cy.ts

@@ -13,21 +13,33 @@ describe('Spec List - Git Status', () => {
   it('shows correct git status for files using real git repo', () => {
     // newly created, not yet committed
     // this is performed by the task `initGitRepoForTestProject`
-    cy.get('[data-cy-row="cypress/e2e/foo.spec.js"]')
+    cy.get('[data-cy-row="foo.spec.js"]')
     .contains('Created')
     .get('[data-cy="git-status-created"]')
 
     // modified by not yet committed
     // this is performed by the task `initGitRepoForTestProject`
-    cy.get('[data-cy-row="cypress/e2e/blank-contents.spec.js"]')
+    cy.get('[data-cy-row="blank-contents.spec.js"]')
     .contains('Modified')
     .get('[data-cy="git-status-modified"]')
 
     // unmodified by current user
     // we still show "modified" but a different style, indicating the last
     // person to touch the file.
-    cy.get('[data-cy-row="cypress/e2e/dom-container.spec.js"]')
+    cy.get('[data-cy-row="dom-container.spec.js"]')
     .contains('Modified')
     .get('[data-cy="git-status-unmodified"]')
+
+    cy.withCtx((ctx) => {
+      ctx.fs.writeFileSync(
+        ctx.path.join(ctx.currentProject!, 'cypress', 'e2e', 'dom-container.spec.js'),
+        '// modifying the spec.',
+      )
+    })
+
+    // should update via GraphQL subscription, now the status is modified.
+    cy.get('[data-cy-row="dom-container.spec.js"]')
+    .contains('Modified')
+    .get('[data-cy="git-status-modified"]')
   })
 })

packages/app/cypress/e2e/subscriptions/specChange-subscription.cy.ts

@@ -135,7 +135,7 @@ module.exports = {
 }`)
         })
 
-        cy.get('[data-cy="spec-item-link"]')
+        cy.get('[data-cy="spec-item-link"]', { timeout: 7500 })
         .should('have.length', 2)
         .should('contain', 'dom-container.spec.js')
         .should('contain', 'dom-content.spec.js')
@@ -250,7 +250,7 @@ module.exports = {
 }`)
         })
 
-        cy.get('[data-testid="spec-file-item"]')
+        cy.get('[data-testid="spec-file-item"]', { timeout: 7500 })
         .should('have.length', 2)
         .should('contain', 'dom-container.spec.js')
         .should('contain', 'dom-content.spec.js')
@@ -352,7 +352,7 @@ module.exports = {
 }`)
         })
 
-        cy.get('[data-cy="file-match-indicator"]')
+        cy.get('[data-cy="file-match-indicator"]', { timeout: 7500 })
         .should('contain', '2 Matches')
       })
     })

packages/app/src/specs/SpecsList.vue

@@ -67,7 +67,7 @@
           :id="getIdIfDirectory(row)"
           :key="row.index"
           :data-cy="row.data.isLeaf ? 'spec-list-file' : 'spec-list-directory'"
-          :data-cy-row="row.data.data?.relative"
+          :data-cy-row="row.data.data?.baseName"
         >
           <template #file>
             <RouterLink

packages/data-context/src/sources/GitDataSource.ts

@@ -35,6 +35,9 @@ const GIT_LOG_COMMAND = `git log -1 --pretty="format:%ci %ar %an"`
 const GIT_ROOT_DIR_COMMAND = '--show-toplevel'
 const SIXTY_SECONDS = 60 * 1000
 
+function ensurePosixPathSeparators (text: string) {
+  return text.replace(/\\/g, '/') // normalize \ to /
+}
 export interface GitInfo {
   author: string | null
   lastModifiedTimestamp: string | null
@@ -315,20 +318,24 @@ export class GitDataSource {
   async #getInfoWindows (absolutePaths: readonly string[]) {
     const paths = absolutePaths.map((x) => path.resolve(x)).join(',')
     const cmd = `FOR %x in (${paths}) DO (${GIT_LOG_COMMAND} %x)`
-    const result = await execa(cmd, { shell: true })
+    const result = await execa(cmd, { shell: true, cwd: this.config.projectRoot })
 
-    const split = result.stdout
-    .split('\r\n') // windows uses CRLF for carriage returns
-    .filter((str) => !str.includes('git log')) // windows stdout contains [cmd,output]. So we remove the code containing the executed command, `git log`
+    const stdout = ensurePosixPathSeparators(result.stdout).split('\r\n') // windows uses CRLF for carriage returns
 
-    // windows returns a leading carriage return, remove it
-    const [, ...stdout] = split
+    const output: string[] = []
 
-    if (stdout.length !== absolutePaths.length) {
-      debug('stdout', stdout)
-      throw Error(`Expect result array to have same length as input. Input: ${absolutePaths.length} Output: ${stdout.length}`)
+    for (const p of absolutePaths) {
+      const idx = stdout.findIndex((entry) => entry.includes(p))
+      const text = stdout[idx + 1] ?? ''
+
+      output.push(text)
     }
 
-    return stdout
+    if (output.length !== absolutePaths.length) {
+      debug('stdout', output)
+      throw Error(`Expect result array to have same length as input. Input: ${absolutePaths.length} Output: ${output.length}`)
+    }
+
+    return output
   }
 }

packages/launchpad/cypress/e2e/migration.cy.ts

@@ -104,7 +104,7 @@ describe('Opening unmigrated project', () => {
   })
 })
 
-describe('Full migration flow for each project', { retries: { openMode: 2, runMode: 2 }, defaultCommandTimeout: 7000 }, () => {
+describe('Full migration flow for each project', { retries: { openMode: 2, runMode: 2 }, defaultCommandTimeout: 10000 }, () => {
   it('completes journey for migration-component-testing', () => {
     startMigrationFor('migration-component-testing')
     // custom testFiles - cannot auto

packages/launchpad/cypress/e2e/project-setup.cy.ts

@@ -26,7 +26,7 @@ describe('Launchpad: Setup Project', () => {
   }
 
   const verifyChooseABrowserPage = () => {
-    cy.contains('Choose a Browser', { timeout: 10000 })
+    cy.contains('Choose a Browser', { timeout: 15000 })
 
     cy.findByRole('radio', { name: 'Chrome v1' })
     cy.findByRole('radio', { name: 'Firefox v5' })