Skip to content

Commit

Permalink
fix: retry on EMFILE always, lint sync FS calls (#22175)
Browse files Browse the repository at this point in the history
* fix: use graceful-fs always, warn in development on sync calls

* skip prop linting in some dirs

* eslint rules

* use AST-based lint rule instead

* comment

* ignore existsSync

* run without nextTick

* remove dev warning code

* fix order

* register TS first

* fix tests

* fix test

* cover new call site

* fix new test
  • Loading branch information
flotwig committed Jun 16, 2022
1 parent e87c492 commit d01932b
Show file tree
Hide file tree
Showing 44 changed files with 161 additions and 192 deletions.
29 changes: 29 additions & 0 deletions .eslintrc.js
Expand Up @@ -5,6 +5,7 @@ const { specifiedRules } = require('graphql')
const graphqlOpts = {
env: 'literal',
tagName: 'gql',
// eslint-disable-next-line no-restricted-syntax
schemaString: fs.readFileSync(
path.join(__dirname, 'packages/graphql/schemas/schema.graphql'),
'utf8',
Expand Down Expand Up @@ -33,6 +34,24 @@ module.exports = {
'plugin:@cypress/dev/tests',
],
parser: '@typescript-eslint/parser',
overrides: [
{
files: [
// ignore in tests and scripts
'**/scripts/**',
'**/test/**',
'**/system-tests/**',
'packages/{app,driver,frontend-shared,launchpad}/cypress/**',
'*.test.ts',
// ignore in packages that don't run in the Cypress process
'npm/create-cypress-tests/**',
],
rules: {
'no-restricted-properties': 'off',
'no-restricted-syntax': 'off',
},
},
],
rules: {
'no-duplicate-imports': 'off',
'import/no-duplicates': 'off',
Expand All @@ -55,6 +74,16 @@ module.exports = {
message: 'os.userInfo() will throw when there is not an `/etc/passwd` entry for the current user (like when running with --user 12345 in Docker). Do not use it unless you catch any potential errors.',
},
],
'no-restricted-syntax': [
// esquery tool: https://estools.github.io/esquery/
'error',
{
// match sync FS methods except for `existsSync`
// examples: fse.readFileSync, fs.readFileSync, this.ctx.fs.readFileSync...
selector: `MemberExpression[object.name='fs'][property.name=/^[A-z]+Sync$/]:not(MemberExpression[property.name='existsSync']), MemberExpression[property.name=/^[A-z]+Sync$/]:not(MemberExpression[property.name='existsSync']):has(MemberExpression[property.name='fs'])`,
message: 'Synchronous fs calls should not be used in Cypress. Use an async API instead.',
},
],
'graphql/capitalized-type-name': ['warn', graphqlOpts],
'graphql/no-deprecated-fields': ['error', graphqlOpts],
'graphql/template-strings': ['error', { ...graphqlOpts, validators }],
Expand Down
1 change: 1 addition & 0 deletions npm/eslint-plugin-dev/lib/custom-rules/index.js
Expand Up @@ -2,6 +2,7 @@ const fs = require('fs')
const path = require('path')

module.exports =
// eslint-disable-next-line no-restricted-syntax
Object.assign({}, ...fs.readdirSync(__dirname)
.filter((filename) => filename.endsWith('.js') && filename !== 'index.js')
.map((filename) => ({ [filename.replace(/\.js$/u, '')]: require(path.resolve(__dirname, filename)) })))
2 changes: 1 addition & 1 deletion npm/vite-dev-server/cypress/e2e/vite-dev-server.cy.ts
Expand Up @@ -38,7 +38,7 @@ describe('Config options', () => {
cy.startAppServer('component')

cy.withCtx(async (ctx, { specWithWhitespace }) => {
ctx.actions.file.writeFileInProject(
await ctx.actions.file.writeFileInProject(
ctx.path.join('src', specWithWhitespace),
await ctx.file.readFileInProject(ctx.path.join('src', 'App.cy.jsx')),
)
Expand Down
2 changes: 2 additions & 0 deletions npm/vite-dev-server/src/plugins/cypress.ts
Expand Up @@ -38,6 +38,8 @@ export const Cypress = (
const indexHtmlFile = options.cypressConfig.indexHtmlFile

let specsPathsSet = getSpecsPathsSet(specs)
// TODO: use async fs methods here
// eslint-disable-next-line no-restricted-syntax
let loader = fs.readFileSync(INIT_FILEPATH, 'utf8')

devServerEvents.on('dev-server:specs:changed', (specs: Spec[]) => {
Expand Down
4 changes: 3 additions & 1 deletion npm/webpack-dev-server/src/CypressCTWebpackPlugin.ts
Expand Up @@ -99,13 +99,15 @@ export class CypressCTWebpackPlugin {
* has been "updated on disk", causing a recompliation (and pulling the new specs in as
* dependencies).
*/
private onSpecsChange = (specs: Cypress.Cypress['spec'][]) => {
private onSpecsChange = async (specs: Cypress.Cypress['spec'][]) => {
if (!this.compilation || _.isEqual(specs, this.files)) {
return
}

this.files = specs
const inputFileSystem = this.compilation.inputFileSystem
// TODO: don't use a sync fs method here
// eslint-disable-next-line no-restricted-syntax
const utimesSync: UtimesSync = inputFileSystem.fileSystem.utimesSync ?? fs.utimesSync

utimesSync(path.resolve(__dirname, 'browser.js'), new Date(), new Date())
Expand Down
12 changes: 6 additions & 6 deletions npm/webpack-dev-server/src/helpers/nextHandler.ts
Expand Up @@ -69,7 +69,7 @@ async function loadWebpackConfig (devServerConfig: WebpackDevServerConfig): Prom
buildId: `@cypress/react-${Math.random().toString()}`,
config: nextConfig,
dev: true,
pagesDir: findPagesDir(devServerConfig.cypressConfig.projectRoot),
pagesDir: await findPagesDir(devServerConfig.cypressConfig.projectRoot),
entrypoints: {},
rewrites: { fallback: [], afterFiles: [], beforeFiles: [] },
...runWebpackSpan,
Expand Down Expand Up @@ -106,9 +106,9 @@ function checkSWC (
return false
}

const existsSync = (file: string) => {
const exists = async (file: string) => {
try {
fs.accessSync(file, fs.constants.F_OK)
await fs.promises.access(file, fs.constants.F_OK)

return true
} catch (_) {
Expand All @@ -121,16 +121,16 @@ const existsSync = (file: string) => {
* `${projectRoot}/pages` or `${projectRoot}/src/pages`.
* If neither is found, return projectRoot
*/
function findPagesDir (projectRoot: string) {
async function findPagesDir (projectRoot: string) {
// prioritize ./pages over ./src/pages
let pagesDir = path.join(projectRoot, 'pages')

if (existsSync(pagesDir)) {
if (await exists(pagesDir)) {
return pagesDir
}

pagesDir = path.join(projectRoot, 'src', 'pages')
if (existsSync(pagesDir)) {
if (await exists(pagesDir)) {
return pagesDir
}

Expand Down
1 change: 1 addition & 0 deletions npm/webpack-dev-server/src/measureWebpackPerformance.ts
@@ -1,3 +1,4 @@
/* eslint-disable no-restricted-syntax */
/* eslint-disable no-console */
import path from 'path'
import fs from 'fs'
Expand Down
2 changes: 1 addition & 1 deletion npm/webpack-preprocessor/package.json
Expand Up @@ -11,7 +11,7 @@
"secure": "nsp check",
"semantic-release": "semantic-release",
"size": "npm pack --dry",
"test": "node ./test-webpack-5.js",
"test": "node ./scripts/test-webpack-5.js",
"test-debug": "node --inspect --debug-brk ./node_modules/.bin/_mocha",
"test-e2e": "mocha test/e2e/*.spec.*",
"test-unit": "mocha test/unit/*.spec.*",
Expand Down
@@ -1,7 +1,10 @@
const execa = require('execa')
const pkg = require('./package.json')
const path = require('path')
const pkg = require('../package.json')
const fs = require('fs')

const pkgJsonPath = path.join(__dirname, '..', 'package.json')

/**
* This file installs Webpack 5 and runs the unit and e2e tests for the preprocessor.
* We read package.json, update the webpack version, then re-run yarn install.
Expand All @@ -17,7 +20,7 @@ const main = async () => {
const install = () => execa('yarn', ['install', '--ignore-scripts'], { stdio: 'inherit' })

const resetPkg = async () => {
fs.writeFileSync('package.json', originalPkg, 'utf8')
fs.writeFileSync(pkgJsonPath, originalPkg, 'utf8')
await install()
}

Expand All @@ -38,7 +41,7 @@ const main = async () => {
delete pkg.devDependencies['webpack']
// eslint-disable-next-line no-console
console.log('[@cypress/webpack-preprocessor]: updating package.json...')
fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2))
fs.writeFileSync(pkgJsonPath, JSON.stringify(pkg, null, 2))

// eslint-disable-next-line no-console
console.log('[@cypress/webpack-preprocessor]: install dependencies...')
Expand Down
4 changes: 2 additions & 2 deletions packages/app/cypress/e2e/cypress-in-cypress-component.cy.ts
Expand Up @@ -208,8 +208,8 @@ describe('Cypress In Cypress CT', { viewportWidth: 1500, defaultCommandTimeout:
}).invoke('connected').should('be.true')
})

cy.withCtx((ctx, o) => {
ctx.actions.file.writeFileInProject(o.path, `
cy.withCtx(async (ctx, o) => {
await ctx.actions.file.writeFileInProject(o.path, `
import React from 'react'
import { mount } from '@cypress/react'
Expand Down
4 changes: 2 additions & 2 deletions packages/app/cypress/e2e/cypress-in-cypress-e2e.cy.ts
Expand Up @@ -259,8 +259,8 @@ describe('Cypress In Cypress E2E', { viewportWidth: 1500, defaultCommandTimeout:
}).invoke('connected').should('be.true')
})

cy.withCtx((ctx, o) => {
ctx.actions.file.writeFileInProject(o.path, `
cy.withCtx(async (ctx, o) => {
await ctx.actions.file.writeFileInProject(o.path, `
describe('Dom Content', () => {
it('renders the new test content', () => {
cy.visit('cypress/e2e/dom-content.html')
Expand Down
30 changes: 15 additions & 15 deletions packages/app/cypress/e2e/cypress-in-cypress.cy.ts
Expand Up @@ -228,13 +228,13 @@ describe('Cypress in Cypress', { viewportWidth: 1500, defaultCommandTimeout: 100
startAtSpecsPage('e2e')
cy.get('[data-cy="spec-item"]')

cy.withCtx((ctx, o) => {
cy.withCtx(async (ctx, o) => {
ctx.coreData.app.browserStatus = 'open'

let config = ctx.actions.file.readFileInProject('cypress.config.js')
let config = await ctx.actions.file.readFileInProject('cypress.config.js')

config = config.replace(`e2e: {`, `e2e: {\n chromeWebSecurity: false,\n`)
ctx.actions.file.writeFileInProject('cypress.config.js', config)
await ctx.actions.file.writeFileInProject('cypress.config.js', config)

o.sinon.stub(ctx.actions.browser, 'closeBrowser')
o.sinon.stub(ctx.actions.browser, 'relaunchBrowser')
Expand All @@ -252,24 +252,24 @@ describe('Cypress in Cypress', { viewportWidth: 1500, defaultCommandTimeout: 100
it('restarts browser if there is a before:browser:launch task and there is a change on the config', () => {
startAtSpecsPage('e2e')

cy.withCtx((ctx, o) => {
cy.withCtx(async (ctx, o) => {
ctx.coreData.app.browserStatus = 'open'

let config = ctx.actions.file.readFileInProject('cypress.config.js')
let config = await ctx.actions.file.readFileInProject('cypress.config.js')

config = config.replace(`e2e: {`, `e2e: {\n setupNodeEvents(on) {\n on('before:browser:launch', () => {})\n},\n`)
ctx.actions.file.writeFileInProject('cypress.config.js', config)
await ctx.actions.file.writeFileInProject('cypress.config.js', config)
})

cy.get('[data-cy="spec-item"]')

cy.withCtx((ctx, o) => {
cy.withCtx(async (ctx, o) => {
ctx.coreData.app.browserStatus = 'open'

let config = ctx.actions.file.readFileInProject('cypress.config.js')
let config = await ctx.actions.file.readFileInProject('cypress.config.js')

config = config.replace(`e2e: {`, `e2e: {\n viewportHeight: 600,\n`)
ctx.actions.file.writeFileInProject('cypress.config.js', config)
await ctx.actions.file.writeFileInProject('cypress.config.js', config)

o.sinon.stub(ctx.actions.browser, 'closeBrowser')
o.sinon.stub(ctx.actions.browser, 'relaunchBrowser')
Expand All @@ -288,14 +288,14 @@ describe('Cypress in Cypress', { viewportWidth: 1500, defaultCommandTimeout: 100
startAtSpecsPage('e2e')
cy.get('[data-cy="spec-item"]')

cy.withCtx((ctx, o) => {
cy.withCtx(async (ctx, o) => {
ctx.coreData.app.browserStatus = 'open'
o.sinon.stub(ctx.actions.project, 'initializeActiveProject')

let config = ctx.actions.file.readFileInProject('cypress.config.js')
let config = await ctx.actions.file.readFileInProject('cypress.config.js')

config = config.replace(`{`, `{\n watchForFileChanges: false,\n`)
ctx.actions.file.writeFileInProject('cypress.config.js', config)
await ctx.actions.file.writeFileInProject('cypress.config.js', config)
})

cy.get('[data-cy="loading-spinner"]').should('be.visible')
Expand All @@ -310,14 +310,14 @@ describe('Cypress in Cypress', { viewportWidth: 1500, defaultCommandTimeout: 100
startAtSpecsPage('e2e')
cy.get('[data-cy="spec-item"]')

cy.withCtx((ctx, o) => {
cy.withCtx(async (ctx, o) => {
ctx.coreData.app.browserStatus = 'open'
o.sinon.stub(ctx.actions.project, 'initializeActiveProject')

let config = ctx.actions.file.readFileInProject('cypress.config.js')
let config = await ctx.actions.file.readFileInProject('cypress.config.js')

config = config.replace(` e2e: {`, ` e2e: {\n baseUrl: 'https://example.cypress.io',\n`)
ctx.actions.file.writeFileInProject('cypress.config.js', config)
await ctx.actions.file.writeFileInProject('cypress.config.js', config)
})

cy.get('[data-cy="loading-spinner"]').should('be.visible')
Expand Down
4 changes: 2 additions & 2 deletions packages/app/cypress/e2e/specs.cy.ts
Expand Up @@ -423,7 +423,7 @@ describe('App: Specs', () => {

it('generates spec with file name that does not contain a known spec extension', () => {
cy.withCtx(async (ctx) => {
let config = ctx.actions.file.readFileInProject('cypress.config.js')
let config = await ctx.actions.file.readFileInProject('cypress.config.js')

config = config.replace(
`specPattern: 'src/**/*.{cy,spec}.{js,jsx}'`,
Expand Down Expand Up @@ -700,7 +700,7 @@ describe('App: Specs', () => {

it('generates spec with file name that does not contain a known spec extension', () => {
cy.withCtx(async (ctx) => {
let config = ctx.actions.file.readFileInProject('cypress.config.js')
let config = await ctx.actions.file.readFileInProject('cypress.config.js')

config = config.replace(
`specPattern: 'src/specs-folder/*.cy.{js,jsx}'`,
Expand Down
@@ -1,18 +1,18 @@
function updateProjectIdInCypressConfig (value: string) {
return cy.withCtx((ctx, o) => {
let config = ctx.actions.file.readFileInProject('cypress.config.js')
return cy.withCtx(async (ctx, o) => {
let config = await ctx.actions.file.readFileInProject('cypress.config.js')

config = config.replace(`projectId: 'abc123'`, `projectId: '${o.value}'`)
ctx.actions.file.writeFileInProject('cypress.config.js', config)
await ctx.actions.file.writeFileInProject('cypress.config.js', config)
}, { value })
}

function updateViewportHeightInCypressConfig (value: number) {
return cy.withCtx((ctx, o) => {
let config = ctx.actions.file.readFileInProject('cypress.config.js')
return cy.withCtx(async (ctx, o) => {
let config = await ctx.actions.file.readFileInProject('cypress.config.js')

config = config.replace(`e2e: {`, `e2e: {\n viewportHeight: ${o.value},\n`)
ctx.actions.file.writeFileInProject('cypress.config.js', config)
await ctx.actions.file.writeFileInProject('cypress.config.js', config)
}, { value })
}

Expand Down
Expand Up @@ -18,8 +18,8 @@ describe('specChange subscription', () => {
.should('contain', 'dom-content.spec.js')
.should('contain', 'dom-list.spec.js')

cy.withCtx((ctx, o) => {
ctx.actions.file.writeFileInProject(o.path, '')
cy.withCtx(async (ctx, o) => {
await ctx.actions.file.writeFileInProject(o.path, '')
}, { path: getPathForPlatform('cypress/e2e/new-file.spec.js') })

cy.get('[data-cy="spec-item-link"]')
Expand Down
16 changes: 8 additions & 8 deletions packages/data-context/src/actions/FileActions.ts
Expand Up @@ -8,29 +8,29 @@ import assert from 'assert'
export class FileActions {
constructor (private ctx: DataContext) {}

readFileInProject (relativePath: string): string {
async readFileInProject (relativePath: string): Promise<string> {
if (!this.ctx.currentProject) {
throw new Error(`Cannot write file in project without active project`)
}

const filePath = path.join(this.ctx.currentProject, relativePath)

this.ctx.fs.ensureDirSync(path.dirname(filePath))
await this.ctx.fs.ensureDir(path.dirname(filePath))

return this.ctx.fs.readFileSync(filePath, 'utf-8')
return this.ctx.fs.readFile(filePath, 'utf-8')
}

writeFileInProject (relativePath: string, data: any) {
async writeFileInProject (relativePath: string, data: any) {
if (!this.ctx.currentProject) {
throw new Error(`Cannot write file in project without active project`)
}

const filePath = path.join(this.ctx.currentProject, relativePath)

this.ctx.fs.ensureDirSync(path.dirname(filePath))
await this.ctx.fs.ensureDir(path.dirname(filePath))

// Typically used in e2e tests, simpler than forcing async
this.ctx.fs.writeFileSync(
await this.ctx.fs.writeFile(
filePath,
data,
)
Expand All @@ -42,7 +42,7 @@ export class FileActions {
}

// Typically used in e2e tests, simpler than forcing async
this.ctx.fs.removeSync(path.join(this.ctx.currentProject, relativePath))
await this.ctx.fs.remove(path.join(this.ctx.currentProject, relativePath))
}

async moveFileInProject (relativePath: string, toRelativePath: string) {
Expand All @@ -51,7 +51,7 @@ export class FileActions {
}

// Typically used in e2e tests, simpler than forcing async
this.ctx.fs.moveSync(
await this.ctx.fs.move(
path.join(this.ctx.currentProject, relativePath),
path.join(this.ctx.currentProject, toRelativePath),
)
Expand Down

5 comments on commit d01932b

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on d01932b Jun 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.2.0/linux-x64/develop-d01932bf751a6edf758451d8d19a74fe07e799ea/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on d01932b Jun 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux arm64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.2.0/linux-arm64/develop-d01932bf751a6edf758451d8d19a74fe07e799ea/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on d01932b Jun 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin arm64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.2.0/darwin-arm64/develop-d01932bf751a6edf758451d8d19a74fe07e799ea/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on d01932b Jun 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.2.0/win32-x64/develop-d01932bf751a6edf758451d8d19a74fe07e799ea/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on d01932b Jun 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/10.2.0/darwin-x64/develop-d01932bf751a6edf758451d8d19a74fe07e799ea/cypress.tgz

Please sign in to comment.