Skip to content

Commit

Permalink
fix: retry on EMFILE always, lint sync FS calls (cypress-io#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 authored and BeijiYang committed Jun 23, 2022
1 parent 83ee9ba commit 05255ca
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

0 comments on commit 05255ca

Please sign in to comment.