-
Notifications
You must be signed in to change notification settings - Fork 12k
run e2e tests in isolated child processes #23245
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
load("//tools:defaults.bzl", "ts_library") | ||
|
||
ts_library( | ||
name = "initialize", | ||
testonly = True, | ||
srcs = glob(["**/*.ts"]), | ||
data = [ | ||
"//:package.json", | ||
], | ||
visibility = ["//visibility:public"], | ||
deps = [ | ||
"//tests/legacy-cli/e2e/utils", | ||
"@npm//@types/yargs-parser", | ||
], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ import { createProjectFromAsset } from '../../../utils/assets'; | |
import { expectFileSizeToBeUnder, expectFileToMatch, replaceInFile } from '../../../utils/fs'; | ||
import { execWithEnv } from '../../../utils/process'; | ||
|
||
export default async function (skipCleaning: () => void) { | ||
export default async function () { | ||
const webpackCLIBin = normalize('node_modules/.bin/webpack-cli'); | ||
|
||
await createProjectFromAsset('webpack/test-app'); | ||
|
@@ -30,6 +30,4 @@ export default async function (skipCleaning: () => void) { | |
'DISABLE_V8_COMPILE_CACHE': '1', | ||
}); | ||
await expectFileToMatch('dist/app.main.js', 'AppModule'); | ||
|
||
skipCleaning(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This didn't seem to effect tests at all? This was the only use case of the "skip cleaning" and removing it didn't cause any tests to start failing. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,26 @@ | ||
const global: { [name: string]: any } = Object.create(null); | ||
const ENV_PREFIX = 'LEGACY_CLI__'; | ||
|
||
export function setGlobalVariable(name: string, value: any) { | ||
global[name] = value; | ||
if (value === undefined) { | ||
delete process.env[ENV_PREFIX + name]; | ||
} else { | ||
process.env[ENV_PREFIX + name] = JSON.stringify(value); | ||
} | ||
} | ||
|
||
export function getGlobalVariable<T = any>(name: string): T { | ||
if (!(name in global)) { | ||
const value = process.env[ENV_PREFIX + name]; | ||
if (value === undefined) { | ||
throw new Error(`Trying to access variable "${name}" but it's not defined.`); | ||
} | ||
return global[name] as T; | ||
return JSON.parse(value) as T; | ||
} | ||
|
||
export function getGlobalVariablesEnv(): NodeJS.ProcessEnv { | ||
return Object.keys(process.env) | ||
.filter((v) => v.startsWith(ENV_PREFIX)) | ||
.reduce<NodeJS.ProcessEnv>((vars, n) => { | ||
vars[n] = process.env[n]; | ||
return vars; | ||
}, {}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,10 @@ import { SpawnOptions } from 'child_process'; | |
import * as child_process from 'child_process'; | ||
import { concat, defer, EMPTY, from } from 'rxjs'; | ||
import { repeat, takeLast } from 'rxjs/operators'; | ||
import { getGlobalVariable } from './env'; | ||
import { getGlobalVariable, getGlobalVariablesEnv } from './env'; | ||
import { catchError } from 'rxjs/operators'; | ||
import treeKill from 'tree-kill'; | ||
import { delimiter, join, resolve } from 'path'; | ||
|
||
interface ExecOptions { | ||
silent?: boolean; | ||
|
@@ -300,22 +301,21 @@ export function silentNpm( | |
{ | ||
silent: true, | ||
cwd: (options as { cwd?: string } | undefined)?.cwd, | ||
env: extractNpmEnv(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None of these are needed anymore because the child process has the correct env passed in already, see |
||
}, | ||
'npm', | ||
params, | ||
); | ||
} else { | ||
return _exec({ silent: true, env: extractNpmEnv() }, 'npm', args as string[]); | ||
return _exec({ silent: true }, 'npm', args as string[]); | ||
} | ||
} | ||
|
||
export function silentYarn(...args: string[]) { | ||
return _exec({ silent: true, env: extractNpmEnv() }, 'yarn', args); | ||
return _exec({ silent: true }, 'yarn', args); | ||
} | ||
|
||
export function npm(...args: string[]) { | ||
return _exec({ env: extractNpmEnv() }, 'npm', args); | ||
return _exec({}, 'npm', args); | ||
} | ||
|
||
export function node(...args: string[]) { | ||
|
@@ -329,3 +329,39 @@ export function git(...args: string[]) { | |
export function silentGit(...args: string[]) { | ||
return _exec({ silent: true }, 'git', args); | ||
} | ||
|
||
/** | ||
* Launch the given entry in an child process isolated to the test environment. | ||
* | ||
* The test environment includes the local NPM registry, isolated NPM globals, | ||
* the PATH variable only referencing the local node_modules and local NPM | ||
* registry (not the test runner or standard global node_modules). | ||
*/ | ||
export async function launchTestProcess(entry: string, ...args: any[]) { | ||
const tempRoot: string = getGlobalVariable('tmp-root'); | ||
|
||
// Extract explicit environment variables for the test process. | ||
const env: NodeJS.ProcessEnv = { | ||
...extractNpmEnv(), | ||
...getGlobalVariablesEnv(), | ||
}; | ||
|
||
// Modify the PATH environment variable... | ||
let paths = process.env.PATH!.split(delimiter); | ||
|
||
// Only include paths within the sandboxed test environment or external | ||
// non angular-cli paths such as /usr/bin for generic commands. | ||
paths = paths.filter((p) => p.startsWith(tempRoot) || !p.includes('angular-cli')); | ||
|
||
// Ensure the custom npm global bin is on the PATH | ||
// https://docs.npmjs.com/cli/v8/configuring-npm/folders#executables | ||
if (process.platform.startsWith('win')) { | ||
paths.unshift(env.NPM_CONFIG_PREFIX!); | ||
} else { | ||
paths.unshift(join(env.NPM_CONFIG_PREFIX!, 'bin')); | ||
} | ||
|
||
env.PATH = paths.join(delimiter); | ||
|
||
return _exec({ env }, process.execPath, [resolve(__dirname, 'run_test_process'), entry, ...args]); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
'use strict'; | ||
require('../../../../lib/bootstrap-local'); | ||
require('./test_process'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { killAllProcesses } from './process'; | ||
|
||
const testScript: string = process.argv[2]; | ||
const testModule = require(testScript); | ||
const testFunction: () => Promise<void> | void = | ||
typeof testModule == 'function' | ||
? testModule | ||
: typeof testModule.default == 'function' | ||
? testModule.default | ||
: () => { | ||
throw new Error('Invalid test module.'); | ||
}; | ||
|
||
(async () => Promise.resolve(testFunction()))() | ||
.finally(killAllProcesses) | ||
.catch((e) => { | ||
console.error(e); | ||
process.exitCode = -1; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this part of the test started failing. I assume something related to the fact that puppeteer and the webdriver (and protractor? I don't recall) are now from within the project instead of from the git repo node_modules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is because the webdriver manager is already update when running https://github.com/angular/angular-cli/pull/23245/files#diff-6f3e609e1aead9e976e142745edcbcf23cbe30c3f61a400146fed645dfd751b2R47-R63
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't it before as well? The postinstall in the root package.json was running it and previously the tests were consuming that. Why is it different now that it's using the webdriver within the test project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's being for E2E's the local package would be used as such the repository package was not being consumed.