Skip to content
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

fix: windows #22234

Merged
merged 12 commits into from
Jun 10, 2022
14 changes: 7 additions & 7 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ executors:
machine:
image: windows-server-2019-vs2019:stable
shell: bash.exe -eo pipefail
resource_class: windows.medium
resource_class: windows.xlarge
environment:
PLATFORM: windows

Expand Down Expand Up @@ -2581,29 +2581,29 @@ windows-workflow: &windows-workflow
- node_modules_install:
name: windows-node-modules-install
executor: windows
resource_class: windows.medium
resource_class: windows.xlarge
only-cache-for-root-user: true

- build:
name: windows-build
context: test-runner:env-canary
executor: windows
resource_class: windows.medium
resource_class: windows.xlarge
requires:
- windows-node-modules-install

- run-app-integration-tests-chrome:
name: windows-run-app-integration-tests-chrome
executor: windows
resource_class: windows.medium
resource_class: windows.xlarge
context: test-runner:launchpad-tests
requires:
- windows-build

- run-launchpad-integration-tests-chrome:
name: windows-run-launchpad-integration-tests-chrome
executor: windows
resource_class: windows.medium
resource_class: windows.xlarge
context: test-runner:launchpad-tests
requires:
- windows-build
Expand All @@ -2619,15 +2619,15 @@ windows-workflow: &windows-workflow
<<: *full-windows-workflow-filters
name: windows-unit-tests
executor: windows
resource_class: windows.medium
resource_class: windows.xlarge
requires:
- windows-build

- create-build-artifacts:
<<: *full-windows-workflow-filters
name: windows-create-build-artifacts
executor: windows
resource_class: windows.medium
resource_class: windows.xlarge
Copy link
Contributor

Choose a reason for hiding this comment

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

A little unsettling that we don't know why this is required, but glad to have windows CI back to green.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. 😕

context:
- test-runner:sign-windows-binary
- test-runner:upload
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('authChange subscription', () => {

it('responds to authChange subscription for login', () => {
cy.contains('Log In')
cy.wait(500)
cy.withCtx(async (ctx) => {
await ctx.actions.auth.login()
})
Expand All @@ -42,6 +43,7 @@ describe('authChange subscription', () => {
setActiveUser()
cy.reload() // Reload to show the latest state
cy.contains('Test User')
cy.wait(500)
cy.withCtx(async (ctx) => {
await ctx.actions.auth.logout()
})
Expand All @@ -58,6 +60,7 @@ describe('authChange subscription', () => {

it('responds to authChange subscription for login', () => {
cy.contains('Log In')
cy.wait(500)
cy.withCtx(async (ctx) => {
await ctx.actions.auth.login()
})
Expand All @@ -69,6 +72,7 @@ describe('authChange subscription', () => {
setActiveUser()
cy.reload() // Reload to show the latest state
cy.contains('Test User')
cy.wait(500)
cy.withCtx(async (ctx) => {
await ctx.actions.auth.logout()
})
Expand All @@ -84,6 +88,7 @@ describe('authChange subscription', () => {

it('responds to authChange subscription for login', () => {
cy.contains('Log In')
cy.wait(500)
cy.withCtx(async (ctx) => {
await ctx.actions.auth.login()
})
Expand All @@ -95,6 +100,7 @@ describe('authChange subscription', () => {
setActiveUser()
cy.visitLaunchpad()
cy.contains('Test User')
cy.wait(500)
cy.withCtx(async (ctx) => {
await ctx.actions.auth.logout()
})
Expand Down
1 change: 1 addition & 0 deletions packages/app/cypress/e2e/top-nav.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ describe('App Top Nav Workflows', () => {
const oldFetch = ctx.util.fetch

o.sinon.stub(ctx.util, 'fetch').callsFake(async (url: RequestInfo | URL, init?: RequestInit) => {
await new Promise((resolve) => setTimeout(resolve, 500))
if (['https://download.cypress.io/desktop.json', 'https://registry.npmjs.org/cypress'].includes(String(url))) {
throw new Error(String(url))
}
Expand Down
20 changes: 0 additions & 20 deletions packages/data-context/src/actions/MigrationActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ import {
} from '../sources/migration'
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify for reviewers: every file change below this line is reverted from #22118 and you don't need to review it to review this PR.

import { makeCoreData } from '../data'
import { LegacyPluginsIpc } from '../data/LegacyPluginsIpc'
import { hasTypeScriptInstalled } from '../util'

const tsNode = require.resolve('@packages/server/lib/plugins/child/register_ts_node')

export function getConfigWithDefaults (legacyConfig: any) {
const newConfig = _.cloneDeep(legacyConfig)
Expand Down Expand Up @@ -91,23 +88,6 @@ export async function processConfigViaLegacyPlugins (projectRoot: string, legacy
const configProcessArgs = ['--projectRoot', projectRoot, '--file', cwd]
const CHILD_PROCESS_FILE_PATH = require.resolve('@packages/server/lib/plugins/child/require_async_child')

// use ts-node if they've got typescript installed
// this matches the 9.x behavior, which is what we want for
// processing legacy pluginsFile (we never supported `"type": "module") in 9.x.
if (hasTypeScriptInstalled(projectRoot)) {
const tsNodeLoader = `--require ${tsNode}`

if (!childOptions.env) {
childOptions.env = {}
}

if (childOptions.env.NODE_OPTIONS) {
childOptions.env.NODE_OPTIONS += ` ${tsNodeLoader}`
} else {
childOptions.env.NODE_OPTIONS = tsNodeLoader
}
}

const childProcess = fork(CHILD_PROCESS_FILE_PATH, configProcessArgs, childOptions)
const ipc = new LegacyPluginsIpc(childProcess)

Expand Down
59 changes: 1 addition & 58 deletions packages/data-context/src/data/ProjectConfigIpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,17 @@ import { CypressError, getError } from '@packages/errors'
import type { FullConfig, TestingType } from '@packages/types'
import { ChildProcess, fork, ForkOptions } from 'child_process'
import EventEmitter from 'events'
import fs from 'fs-extra'
import path from 'path'
import inspector from 'inspector'
import debugLib from 'debug'
import { autoBindDebug, hasTypeScriptInstalled } from '../util'
import { autoBindDebug } from '../util'
import _ from 'lodash'

const pkg = require('@packages/root')
const debug = debugLib(`cypress:lifecycle:ProjectConfigIpc`)

const CHILD_PROCESS_FILE_PATH = require.resolve('@packages/server/lib/plugins/child/require_async_child')

const tsNodeEsm = require.resolve('ts-node/esm/transpile-only')
const tsNode = require.resolve('@packages/server/lib/plugins/child/register_ts_node')

export type IpcHandler = (ipc: ProjectConfigIpc) => void

export interface SetupNodeEventsReply {
Expand Down Expand Up @@ -242,59 +238,6 @@ export class ProjectConfigIpc extends EventEmitter {

debug('fork child process %o', { CHILD_PROCESS_FILE_PATH, configProcessArgs, childOptions: _.omit(childOptions, 'env') })

let isProjectUsingESModules = false

try {
const pkgJson = fs.readJsonSync(path.join(this.projectRoot, 'package.json'))

isProjectUsingESModules = pkgJson.type === 'module'
} catch (e) {
// project does not have `package.json` or it was not found
// reasonable to assume not using es modules
}

if (!childOptions.env) {
childOptions.env = {}
}

// If they've got TypeScript installed, we can use
// ts-node for CommonJS
// ts-node/esm for ESM
if (hasTypeScriptInstalled(this.projectRoot)) {
if (isProjectUsingESModules) {
// Use the ts-node/esm loader so they can use TypeScript with `"type": "module".
// The loader API is experimental and will change.
// The same can be said for the other alternative, esbuild, so this is the
// best option that leverages the existing modules we bundle in the binary.
// @see ts-node esm loader https://typestrong.org/ts-node/docs/usage/#node-flags-and-other-tools
// @see Node.js Loader API https://nodejs.org/api/esm.html#customizing-esm-specifier-resolution-algorithm
const tsNodeEsmLoader = `--experimental-specifier-resolution=node --loader ${tsNodeEsm}`

if (childOptions.env.NODE_OPTIONS) {
childOptions.env.NODE_OPTIONS += ` ${tsNodeEsmLoader}`
} else {
childOptions.env.NODE_OPTIONS = tsNodeEsmLoader
}
} else {
// Not using ES Modules (via "type": "module"),
// so we just register the standard ts-node module
// to handle TypeScript that is compiled to CommonJS.
// We do NOT use the `--loader` flag because we have some additional
// custom logic for ts-node when used with CommonJS that needs to be evaluated
// so we need to load and evaluate the hook first using the `--require` module API.
const tsNodeLoader = `--require ${tsNode}`

if (childOptions.env.NODE_OPTIONS) {
childOptions.env.NODE_OPTIONS += ` ${tsNodeLoader}`
} else {
childOptions.env.NODE_OPTIONS = tsNodeLoader
}
}
} else {
// Just use Node's built-in ESM support.
// TODO: Consider using userland `esbuild` with Node's --loader API to handle ESM.
}

const proc = fork(CHILD_PROCESS_FILE_PATH, configProcessArgs, childOptions)

return proc
Expand Down
15 changes: 4 additions & 11 deletions packages/data-context/src/data/ProjectConfigManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,17 +260,10 @@ export class ProjectConfigManager {
this._eventsIpc.cleanupIpc()
}

this._eventsIpc = new ProjectConfigIpc(
this.options.ctx.nodePath,
this.options.projectRoot,
this.configFilePath,
this.options.configFile,
(cypressError: CypressError, title?: string | undefined) => {
this._state = 'errored'
this.options.ctx.onError(cypressError, title)
},
this.options.ctx.onWarning,
)
this._eventsIpc = new ProjectConfigIpc(this.options.ctx.nodePath, this.options.projectRoot, this.configFilePath, this.options.configFile, (cypressError: CypressError, title?: string | undefined) => {
this._state = 'errored'
this.options.ctx.onError(cypressError, title)
}, this.options.ctx.onWarning)

this._loadConfigPromise = this._eventsIpc.loadConfig()
}
Expand Down
9 changes: 0 additions & 9 deletions packages/data-context/src/util/hasTypescript.ts

This file was deleted.

1 change: 0 additions & 1 deletion packages/data-context/src/util/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@ export * from './autoBindDebug'
export * from './cached'
export * from './config-file-updater'
export * from './file'
export * from './hasTypescript'
export * from './pluginHandlers'
export * from './urqlCacheKeys'
18 changes: 0 additions & 18 deletions packages/data-context/test/unit/util/hasTypescript.spec.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ describe('Launchpad: Error System Tests', () => {
cy.contains('h1', cy.i18n.launchpadErrors.generic.configErrorTitle)
cy.percySnapshot()

cy.get('[data-testid="error-code-frame"]').should('contain', 'cypress.config.ts:6:10')
cy.get('[data-testid="error-code-frame"]').should('contain', 'cypress.config.ts:6:9')
})
})

Expand Down
2 changes: 1 addition & 1 deletion packages/launchpad/cypress/e2e/error-handling.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('Error handling', () => {
cy.contains('Choose a Browser').should('not.exist')

cy.withCtx((ctx) => {
ctx.actions.file.writeFileInProject('cypress.config.ts', `
ctx.actions.file.writeFileInProject('cypress.config.js', `
import { defineConfig } from 'cypress'
import { defineConfig as viteConfig } from 'vite'
export default defineConfig({
Expand Down
1 change: 1 addition & 0 deletions packages/launchpad/cypress/e2e/slow-network.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe('slow network: launchpad', () => {
cy.visitLaunchpad()
cy.get('[data-cy=top-nav-cypress-version-current-link]').should('not.exist')
cy.contains('Log In')
cy.wait(500)
cy.withCtx(async (ctx, o) => {
o.testState.pendingFetches.map((f) => f.resolve())
})
Expand Down
16 changes: 0 additions & 16 deletions packages/server/lib/plugins/child/register_ts_node.js

This file was deleted.

Loading