fix(docker): do not pollute stdout when used with JSON reporter (#17315)

This patch moves parts of docker configuration to a plugin so that
it can rely on repoter for stdout.

Drive-by: provide all plugins with reporter in the `setup` callback.
This commit is contained in:
Andrey Lushnikov 2022-09-13 17:05:37 -07:00 committed by GitHub
parent 881f3101bd
commit dfcd2a273d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 65 additions and 47 deletions

View File

@ -100,9 +100,8 @@ function addTestCommand(program: Command, isDocker: boolean) {
command.option('-x', `Stop after the first failure`);
command.action(async (args, opts) => {
try {
isDocker = isDocker || !!process.env.PLAYWRIGHT_DOCKER;
if (isDocker && !process.env.PW_TS_ESM_ON)
await docker.configureTestRunnerToUseDocker();
if (isDocker)
process.env.PLAYWRIGHT_DOCKER = '1';
await runTests(args, opts);
} catch (e) {
console.error(e);

View File

@ -22,6 +22,8 @@ import { spawnAsync } from 'playwright-core/lib/utils/spawnAsync';
import * as utils from 'playwright-core/lib/utils';
import { getPlaywrightVersion } from 'playwright-core/lib/common/userAgent';
import * as dockerApi from './dockerApi';
import type { TestRunnerPlugin } from '../plugins';
import type { FullConfig, Reporter, Suite } from '../../types/testReporter';
const VRT_IMAGE_DISTRO = 'focal';
const VRT_IMAGE_NAME = `playwright:local-${getPlaywrightVersion()}-${VRT_IMAGE_DISTRO}`;
@ -128,28 +130,37 @@ export async function buildPlaywrightImage() {
console.log(`Done!`);
}
export async function configureTestRunnerToUseDocker() {
console.log(colors.dim('Using docker container to run browsers.'));
await checkDockerEngineIsRunningOrDie();
let info = await containerInfo();
if (!info) {
process.stdout.write(colors.dim(`Starting docker container... `));
const time = Date.now();
info = await ensurePlaywrightContainerOrDie();
const deltaMs = (Date.now() - time);
console.log(colors.dim('Done in ' + (deltaMs / 1000).toFixed(1) + 's'));
console.log(colors.dim('The Docker container will keep running after tests finished.'));
console.log(colors.dim('Stop manually using:'));
console.log(colors.dim(' npx playwright docker stop'));
}
console.log(colors.dim(`View screen: ${info.vncSession}`));
process.env.PW_TEST_CONNECT_WS_ENDPOINT = info.wsEndpoint;
process.env.PW_TEST_CONNECT_HEADERS = JSON.stringify({
'x-playwright-proxy': '*',
});
process.env.PLAYWRIGHT_DOCKER = '1';
}
export const dockerPlugin: TestRunnerPlugin = {
name: 'playwright:docker',
async setup(config: FullConfig, configDir: string, rootSuite: Suite, reporter: Reporter) {
if (!process.env.PLAYWRIGHT_DOCKER)
return;
const print = (text: string) => reporter.onStdOut?.(text);
const println = (text: string) => reporter.onStdOut?.(text + '\n');
println(colors.dim('Using docker container to run browsers.'));
await checkDockerEngineIsRunningOrDie();
let info = await containerInfo();
if (!info) {
print(colors.dim(`Starting docker container... `));
const time = Date.now();
info = await ensurePlaywrightContainerOrDie();
const deltaMs = (Date.now() - time);
println(colors.dim('Done in ' + (deltaMs / 1000).toFixed(1) + 's'));
println(colors.dim('The Docker container will keep running after tests finished.'));
println(colors.dim('Stop manually using:'));
println(colors.dim(' npx playwright docker stop'));
}
println(colors.dim(`View screen: ${info.vncSession}`));
println('');
process.env.PW_TEST_CONNECT_WS_ENDPOINT = info.wsEndpoint;
process.env.PW_TEST_CONNECT_HEADERS = JSON.stringify({
'x-playwright-proxy': '*',
});
},
};
interface ContainerInfo {
wsEndpoint: string;

View File

@ -14,13 +14,13 @@
* limitations under the License.
*/
import type { Suite } from '../../types/testReporter';
import type { Suite, Reporter } from '../../types/testReporter';
import type { Runner } from '../runner';
import type { FullConfig } from '../types';
export interface TestRunnerPlugin {
name: string;
setup?(config: FullConfig, configDir: string, rootSuite: Suite): Promise<void>;
setup?(config: FullConfig, configDir: string, rootSuite: Suite, reporter: Reporter): Promise<void>;
teardown?(): Promise<void>;
}
@ -28,15 +28,18 @@ export { webServer } from './webServerPlugin';
export { gitCommitInfo } from './gitCommitInfoPlugin';
let runnerInstanceToAddPluginsTo: Runner | undefined;
const deferredPlugins: TestRunnerPlugin[] = [];
export const setRunnerToAddPluginsTo = (runner: Runner) => {
runnerInstanceToAddPluginsTo = runner;
for (const plugin of deferredPlugins)
runnerInstanceToAddPluginsTo.addPlugin(plugin);
};
export const addRunnerPlugin = (plugin: TestRunnerPlugin | (() => TestRunnerPlugin)) => {
// Only present in runner, absent in worker.
if (runnerInstanceToAddPluginsTo) {
plugin = typeof plugin === 'function' ? plugin() : plugin;
plugin = typeof plugin === 'function' ? plugin() : plugin;
if (runnerInstanceToAddPluginsTo)
runnerInstanceToAddPluginsTo.addPlugin(plugin);
}
else
deferredPlugins.push(plugin);
};

View File

@ -22,7 +22,7 @@ import { debug } from 'playwright-core/lib/utilsBundle';
import { raceAgainstTimeout } from 'playwright-core/lib/utils/timeoutRunner';
import { launchProcess } from 'playwright-core/lib/utils/processLauncher';
import type { FullConfig, Reporter } from '../../types/testReporter';
import type { FullConfig, Reporter, Suite } from '../../types/testReporter';
import type { TestRunnerPlugin } from '.';
import type { FullConfigInternal } from '../types';
import { envWithoutExperimentalLoaderOptions } from '../cli';
@ -45,21 +45,22 @@ const DEFAULT_ENVIRONMENT_VARIABLES = {
const debugWebServer = debug('pw:webserver');
export class WebServerPlugin implements TestRunnerPlugin {
private _isAvailable: () => Promise<boolean>;
private _isAvailable?: () => Promise<boolean>;
private _killProcess?: () => Promise<void>;
private _processExitedPromise!: Promise<any>;
private _options: WebServerPluginOptions;
private _reporter: Reporter;
private _checkPortOnly: boolean;
private _reporter?: Reporter;
name = 'playwright:webserver';
constructor(options: WebServerPluginOptions, checkPortOnly: boolean, reporter: Reporter) {
this._reporter = reporter;
constructor(options: WebServerPluginOptions, checkPortOnly: boolean) {
this._options = options;
this._isAvailable = getIsAvailableFunction(options.url, checkPortOnly, !!options.ignoreHTTPSErrors, this._reporter.onStdErr?.bind(this._reporter));
this._checkPortOnly = checkPortOnly;
}
public async setup(config: FullConfig, configDir: string) {
public async setup(config: FullConfig, configDir: string, rootSuite: Suite, reporter: Reporter) {
this._reporter = reporter;
this._isAvailable = getIsAvailableFunction(this._options.url, this._checkPortOnly, !!this._options.ignoreHTTPSErrors, this._reporter.onStdErr?.bind(this._reporter));
this._options.cwd = this._options.cwd ? path.resolve(configDir, this._options.cwd) : configDir;
try {
await this._startProcess();
@ -78,7 +79,7 @@ export class WebServerPlugin implements TestRunnerPlugin {
let processExitedReject = (error: Error) => { };
this._processExitedPromise = new Promise((_, reject) => processExitedReject = reject);
const isAlreadyAvailable = await this._isAvailable();
const isAlreadyAvailable = await this._isAvailable!();
if (isAlreadyAvailable) {
debugWebServer(`WebServer is already available`);
if (this._options.reuseExistingServer)
@ -107,10 +108,10 @@ export class WebServerPlugin implements TestRunnerPlugin {
debugWebServer(`Process started`);
launchedProcess.stderr!.on('data', line => this._reporter.onStdErr?.('[WebServer] ' + line.toString()));
launchedProcess.stderr!.on('data', line => this._reporter!.onStdErr?.('[WebServer] ' + line.toString()));
launchedProcess.stdout!.on('data', line => {
if (debugWebServer.enabled)
this._reporter.onStdOut?.('[WebServer] ' + line.toString());
this._reporter!.onStdOut?.('[WebServer] ' + line.toString());
});
}
@ -124,7 +125,7 @@ export class WebServerPlugin implements TestRunnerPlugin {
const launchTimeout = this._options.timeout || 60 * 1000;
const cancellationToken = { canceled: false };
const { timedOut } = (await Promise.race([
raceAgainstTimeout(() => waitFor(this._isAvailable, cancellationToken), launchTimeout),
raceAgainstTimeout(() => waitFor(this._isAvailable!, cancellationToken), launchTimeout),
this._processExitedPromise,
]));
cancellationToken.canceled = true;
@ -203,10 +204,10 @@ function getIsAvailableFunction(url: string, checkPortOnly: boolean, ignoreHTTPS
export const webServer = (options: WebServerPluginOptions): TestRunnerPlugin => {
// eslint-disable-next-line no-console
return new WebServerPlugin(options, false, { onStdOut: d => console.log(d.toString()), onStdErr: d => console.error(d.toString()) });
return new WebServerPlugin(options, false);
};
export const webServerPluginsForConfig = (config: FullConfigInternal, reporter: Reporter): TestRunnerPlugin[] => {
export const webServerPluginsForConfig = (config: FullConfigInternal): TestRunnerPlugin[] => {
const shouldSetBaseUrl = !!config.webServer;
const webServerPlugins = [];
for (const webServerConfig of config._webServers) {
@ -219,7 +220,7 @@ export const webServerPluginsForConfig = (config: FullConfigInternal, reporter:
if (shouldSetBaseUrl && !webServerConfig.url)
process.env.PLAYWRIGHT_TEST_BASE_URL = url;
webServerPlugins.push(new WebServerPlugin({ ...webServerConfig, url }, webServerConfig.port !== undefined, reporter));
webServerPlugins.push(new WebServerPlugin({ ...webServerConfig, url }, webServerConfig.port !== undefined));
}
return webServerPlugins;

View File

@ -44,6 +44,7 @@ import { SigIntWatcher } from './sigIntWatcher';
import type { TestRunnerPlugin } from './plugins';
import { setRunnerToAddPluginsTo } from './plugins';
import { webServerPluginsForConfig } from './plugins/webServerPlugin';
import { dockerPlugin } from './docker/docker';
import { MultiMap } from 'playwright-core/lib/utils/multimap';
const removeFolderAsync = promisify(rimraf);
@ -603,14 +604,17 @@ export class Runner {
};
// Legacy webServer support.
this._plugins.push(...webServerPluginsForConfig(config, this._reporter));
this._plugins.push(...webServerPluginsForConfig(config));
// Docker support.
this._plugins.push(dockerPlugin);
await this._runAndReportError(async () => {
// First run the plugins, if plugin is a web server we want it to run before the
// config's global setup.
for (const plugin of this._plugins) {
await Promise.race([
plugin.setup?.(config, config._configDir, rootSuite),
plugin.setup?.(config, config._configDir, rootSuite, this._reporter),
sigintWatcher.promise(),
]);
if (sigintWatcher.hadSignal())