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.

Sign up for GitHub

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

Jump to bottom

chore: read config file location from url params in ui mode #30274

Closed
wants to merge 1 commit into from
Closed

chore: read config file location from url params in ui mode #30274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/playwright-core/src/server/trace/viewer/traceViewer.ts
View file
Open in desktop
Expand Up @@ -105,7 +105,7 @@ export async function startTraceViewerServer(options?: TraceViewerServerOptions)
return server;
}

export async function installRootRedirect(server: HttpServer, traceUrls: string[], options: TraceViewerRedirectOptions) {
export async function installRootRedirect(server: HttpServer, traceUrls: string[], options: TraceViewerRedirectOptions & { resolvedConfigFile?: string }) {
const params = new URLSearchParams();
for (const traceUrl of traceUrls)
params.append('trace', traceUrl);
Expand All @@ -115,6 +115,8 @@ export async function installRootRedirect(server: HttpServer, traceUrls: string[
params.append('isServer', '');
if (isUnderTest())
params.append('isUnderTest', 'true');
if (options.resolvedConfigFile)
params.append('resolvedConfigFile', path.resolve(options.resolvedConfigFile));
for (const arg of options.args || [])
params.append('arg', arg);
if (options.grep)
Expand Down
26 changes: 14 additions & 12 deletions packages/playwright/src/common/configLoader.ts
View file
Open in desktop
Expand Up @@ -286,7 +286,16 @@ function validateProject(file: string, project: Project, title: string) {
}
}

export function resolveConfigFile(configFileOrDirectory: string): string | undefined {
export function resolveConfigLocation(configFile: string | undefined): ConfigLocation {
const configFileOrDirectory = configFile ? path.resolve(process.cwd(), configFile) : process.cwd();
const resolvedConfigFile = resolveConfigFile(configFileOrDirectory);
return {
resolvedConfigFile,
configDir: resolvedConfigFile ? path.dirname(resolvedConfigFile) : configFileOrDirectory,
};
}

function resolveConfigFile(configFileOrDirectory: string): string | undefined {
const resolveConfig = (configFile: string) => {
if (fs.existsSync(configFile))
return configFile;
Expand All @@ -309,22 +318,15 @@ export function resolveConfigFile(configFileOrDirectory: string): string | undef
return configFile;
// If there is no config, assume this as a root testing directory.
return undefined;
} else {
// When passed a file, it must be a config file.
const configFile = resolveConfig(configFileOrDirectory);
return configFile!;
}
// When passed a file, it must be a config file.
return configFileOrDirectory!;
}

export async function loadConfigFromFileRestartIfNeeded(configFile: string | undefined, overrides?: ConfigCLIOverrides, ignoreDeps?: boolean): Promise<FullConfigInternal | null> {
const configFileOrDirectory = configFile ? path.resolve(process.cwd(), configFile) : process.cwd();
const resolvedConfigFile = resolveConfigFile(configFileOrDirectory);
if (restartWithExperimentalTsEsm(resolvedConfigFile))
const location = resolveConfigLocation(configFile);
if (restartWithExperimentalTsEsm(location.resolvedConfigFile))
return null;
const location: ConfigLocation = {
configDir: resolvedConfigFile ? path.dirname(resolvedConfigFile) : configFileOrDirectory,
resolvedConfigFile,
};
return await loadConfig(location, overrides, ignoreDeps);
}

Expand Down
49 changes: 24 additions & 25 deletions packages/playwright/src/runner/testServer.ts
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@ import { ManualPromise, gracefullyProcessExitDoNotHang, isUnderTest } from 'play
import type { Transport, HttpServer } from 'playwright-core/lib/utils';
import type * as reporterTypes from '../../types/testReporter';
import { collectAffectedTestFiles, dependenciesForTestFile } from '../transform/compilationCache';
import type { FullConfigInternal } from '../common/config';
import type { ConfigLocation, FullConfigInternal } from '../common/config';
import { InternalReporter } from '../reporters/internalReporter';
import { createReporterForTestServer, createReporters } from './reporters';
import { TestRun, createTaskRunnerForList, createTaskRunnerForTestServer, createTaskRunnerForWatchSetup, createTaskRunnerForListFiles } from './tasks';
Expand All @@ -33,7 +33,7 @@ import { Watcher } from '../fsWatcher';
import type { ReportEntry, TestServerInterface, TestServerInterfaceEventEmitters } from '../isomorphic/testServerInterface';
import { Runner } from './runner';
import type { ConfigCLIOverrides } from '../common/ipc';
import { loadConfig, resolveConfigFile, restartWithExperimentalTsEsm } from '../common/configLoader';
import { loadConfig, resolveConfigLocation, restartWithExperimentalTsEsm } from '../common/configLoader';
import { webServerPluginsForConfig } from '../plugins/webServerPlugin';
import type { TraceViewerRedirectOptions, TraceViewerServerOptions } from 'playwright-core/lib/server/trace/viewer/traceViewer';
import type { TestRunnerPluginRegistration } from '../plugins';
Expand All @@ -44,15 +44,15 @@ const originalStdoutWrite = process.stdout.write;
const originalStderrWrite = process.stderr.write;

class TestServer {
private _configFile: string | undefined;
private _configLocation: ConfigLocation;
private _dispatcher: TestServerDispatcher | undefined;

constructor(configFile: string | undefined) {
this._configFile = configFile;
constructor(configLocation: ConfigLocation) {
this._configLocation = configLocation;
}

async start(options: { host?: string, port?: number }): Promise<HttpServer> {
this._dispatcher = new TestServerDispatcher(this._configFile);
this._dispatcher = new TestServerDispatcher(this._configLocation);
return await startTraceViewerServer({ ...options, transport: this._dispatcher.transport });
}

Expand All @@ -63,7 +63,7 @@ class TestServer {
}

class TestServerDispatcher implements TestServerInterface {
private _configFile: string | undefined;
private _configLocation: ConfigLocation;
private _globalWatcher: Watcher;
private _testWatcher: Watcher;
private _testRun: { run: Promise<reporterTypes.FullResult['status']>, stop: ManualPromise<void> } | undefined;
Expand All @@ -77,8 +77,8 @@ class TestServerDispatcher implements TestServerInterface {
private _closeOnDisconnect = false;
private _devServerHandle: (() => Promise<void>) | undefined;

constructor(configFile: string | undefined) {
this._configFile = configFile;
constructor(configLocation: ConfigLocation) {
this._configLocation = configLocation;
this.transport = {
dispatch: (method, params) => (this as any)[method](params),
onclose: () => {
Expand Down Expand Up @@ -145,7 +145,7 @@ class TestServerDispatcher implements TestServerInterface {
await this.runGlobalTeardown();

const { reporter, report } = await this._collectingReporter();
const { config, error } = await this._loadConfig(this._configFile);
const { config, error } = await this._loadConfig();
if (!config) {
reporter.onError(error!);
return { status: 'failed', report };
Expand Down Expand Up @@ -178,7 +178,7 @@ class TestServerDispatcher implements TestServerInterface {
if (this._devServerHandle)
return { status: 'failed', report: [] };
const { reporter, report } = await this._collectingReporter();
const { config, error } = await this._loadConfig(this._configFile);
const { config, error } = await this._loadConfig();
if (!config) {
reporter.onError(error!);
return { status: 'failed', report };
Expand Down Expand Up @@ -212,14 +212,14 @@ class TestServerDispatcher implements TestServerInterface {
}

async clearCache(params: Parameters<TestServerInterface['clearCache']>[0]): ReturnType<TestServerInterface['clearCache']> {
const { config } = await this._loadConfig(this._configFile);
const { config } = await this._loadConfig();
if (config)
await clearCacheAndLogToConsole(config);
}

async listFiles(params: Parameters<TestServerInterface['listFiles']>[0]): ReturnType<TestServerInterface['listFiles']> {
const { reporter, report } = await this._collectingReporter();
const { config, error } = await this._loadConfig(this._configFile);
const { config, error } = await this._loadConfig();
if (!config) {
reporter.onError(error!);
return { status: 'failed', report };
Expand Down Expand Up @@ -250,7 +250,7 @@ class TestServerDispatcher implements TestServerInterface {
retries: 0,
};
const { reporter, report } = await this._collectingReporter();
const { config, error } = await this._loadConfig(this._configFile, overrides);
const { config, error } = await this._loadConfig(overrides);
if (!config) {
reporter.onError(error!);
return { report: [], status: 'failed' };
Expand Down Expand Up @@ -315,7 +315,7 @@ class TestServerDispatcher implements TestServerInterface {
else
process.env.PW_LIVE_TRACE_STACKS = undefined;

const { config, error } = await this._loadConfig(this._configFile, overrides);
const { config, error } = await this._loadConfig(overrides);
if (!config) {
const wireReporter = await this._wireReporter(e => this._dispatchEvent('report', e));
wireReporter.onError(error!);
Expand Down Expand Up @@ -359,7 +359,7 @@ class TestServerDispatcher implements TestServerInterface {
}

async findRelatedTestFiles(params: Parameters<TestServerInterface['findRelatedTestFiles']>[0]): ReturnType<TestServerInterface['findRelatedTestFiles']> {
const { config, error } = await this._loadConfig(this._configFile);
const { config, error } = await this._loadConfig();
if (error)
return { testFiles: [], errors: [error] };
const runner = new Runner(config!);
Expand Down Expand Up @@ -393,11 +393,9 @@ class TestServerDispatcher implements TestServerInterface {
gracefullyProcessExitDoNotHang(0);
}

private async _loadConfig(configFile: string | undefined, overrides?: ConfigCLIOverrides): Promise<{ config: FullConfigInternal | null, error?: reporterTypes.TestError }> {
const configFileOrDirectory = configFile ? path.resolve(process.cwd(), configFile) : process.cwd();
const resolvedConfigFile = resolveConfigFile(configFileOrDirectory);
private async _loadConfig(overrides?: ConfigCLIOverrides): Promise<{ config: FullConfigInternal | null, error?: reporterTypes.TestError }> {
try {
const config = await loadConfig({ resolvedConfigFile, configDir: resolvedConfigFile === configFileOrDirectory ? path.dirname(resolvedConfigFile) : configFileOrDirectory }, overrides);
const config = await loadConfig(this._configLocation, overrides);
// Preserve plugin instances between setup and build.
if (!this._plugins)
this._plugins = config.plugins || [];
Expand All @@ -411,8 +409,8 @@ class TestServerDispatcher implements TestServerInterface {
}

export async function runUIMode(configFile: string | undefined, options: TraceViewerServerOptions & TraceViewerRedirectOptions): Promise<reporterTypes.FullResult['status'] | 'restarted'> {
return await innerRunTestServer(configFile, options, async (server: HttpServer, cancelPromise: ManualPromise<void>) => {
await installRootRedirect(server, [], { ...options, webApp: 'uiMode.html' });
return await innerRunTestServer(configFile, options, async (server: HttpServer, cancelPromise: ManualPromise<void>, configLocation: ConfigLocation) => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: resolve configFile explicitly instead of fetching it back via the callback

await installRootRedirect(server, [], { ...options, webApp: 'uiMode.html', resolvedConfigFile: configLocation.resolvedConfigFile });
if (options.host !== undefined || options.port !== undefined) {
await openTraceInBrowser(server.urlPrefix());
} else {
Expand All @@ -434,17 +432,18 @@ export async function runTestServer(configFile: string | undefined, options: { h
});
}

async function innerRunTestServer(configFile: string | undefined, options: { host?: string, port?: number }, openUI: (server: HttpServer, cancelPromise: ManualPromise<void>) => Promise<void>): Promise<reporterTypes.FullResult['status'] | 'restarted'> {
async function innerRunTestServer(configFile: string | undefined, options: { host?: string, port?: number }, openUI: (server: HttpServer, cancelPromise: ManualPromise<void>, configLocation: ConfigLocation) => Promise<void>): Promise<reporterTypes.FullResult['status'] | 'restarted'> {
if (restartWithExperimentalTsEsm(undefined, true))
return 'restarted';
const testServer = new TestServer(configFile);
const configLocation = resolveConfigLocation(configFile);
const testServer = new TestServer(configLocation);
const cancelPromise = new ManualPromise<void>();
const sigintWatcher = new SigIntWatcher();
process.stdin.on('close', () => gracefullyProcessExitDoNotHang(0));
void sigintWatcher.promise().then(() => cancelPromise.resolve());
try {
const server = await testServer.start(options);
await openUI(server, cancelPromise);
await openUI(server, cancelPromise, configLocation);
await cancelPromise;
} finally {
await testServer.stop();
Expand Down
10 changes: 4 additions & 6 deletions packages/trace-viewer/src/ui/uiModeFiltersView.tsx
View file
Open in desktop
Expand Up @@ -20,7 +20,6 @@ import '@web/third_party/vscode/codicon.css';
import { settings } from '@web/uiUtils';
import React from 'react';
import './uiModeFiltersView.css';
import type { TestModel } from './uiModeModel';

export const FiltersView: React.FC<{
filterText: string;
Expand All @@ -29,9 +28,9 @@ export const FiltersView: React.FC<{
setStatusFilters: (filters: Map<string, boolean>) => void;
projectFilters: Map<string, boolean>;
setProjectFilters: (filters: Map<string, boolean>) => void;
testModel: TestModel | undefined,
resolvedConfigFile: string | undefined,
runTests: () => void;
}> = ({ filterText, setFilterText, statusFilters, setStatusFilters, projectFilters, setProjectFilters, testModel, runTests }) => {
}> = ({ filterText, setFilterText, statusFilters, setStatusFilters, projectFilters, setProjectFilters, resolvedConfigFile, runTests }) => {
const [expanded, setExpanded] = React.useState(false);
const inputRef = React.useRef<HTMLInputElement>(null);
React.useEffect(() => {
Expand Down Expand Up @@ -80,9 +79,8 @@ export const FiltersView: React.FC<{
const copy = new Map(projectFilters);
copy.set(projectName, !copy.get(projectName));
setProjectFilters(copy);
const configFile = testModel?.config?.configFile;
Copy link
Member

Choose a reason for hiding this comment

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

It is safe to use testModel.config.configFile - it will always be there in the UI mode scenario.

if (configFile)
settings.setObject(configFile + ':projects', [...copy.entries()].filter(([_, v]) => v).map(([k]) => k));
if (resolvedConfigFile)
settings.setObject(resolvedConfigFile + ':projects', [...copy.entries()].filter(([_, v]) => v).map(([k]) => k));
}}/>
<div>{projectName || 'untitled'}</div>
</label>
Expand Down
7 changes: 4 additions & 3 deletions packages/trace-viewer/src/ui/uiModeView.tsx
View file
Open in desktop
Expand Up @@ -53,6 +53,7 @@ const guid = searchParams.get('ws');
const wsURL = new URL(`../${guid}`, window.location.toString());
wsURL.protocol = (window.location.protocol === 'https:' ? 'wss:' : 'ws:');
const queryParams = {
resolvedConfigFile: searchParams.get('resolvedConfigFile') || undefined,
args: searchParams.getAll('arg'),
grep: searchParams.get('grep') || undefined,
grepInvert: searchParams.get('grepInvert') || undefined,
Expand Down Expand Up @@ -201,8 +202,8 @@ export const UIModeView: React.FC<{}> = ({
if (!testModel)
return;

const { config, rootSuite } = testModel;
const selectedProjects = config.configFile ? settings.getObject<string[] | undefined>(config.configFile + ':projects', undefined) : undefined;
const { rootSuite } = testModel;
const selectedProjects = queryParams.resolvedConfigFile ? settings.getObject<string[] | undefined>(queryParams.resolvedConfigFile + ':projects', undefined) : undefined;
const newFilter = new Map(projectFilters);
for (const projectName of newFilter.keys()) {
if (!rootSuite.suites.find(s => s.title === projectName))
Expand Down Expand Up @@ -411,7 +412,7 @@ export const UIModeView: React.FC<{}> = ({
setStatusFilters={setStatusFilters}
projectFilters={projectFilters}
setProjectFilters={setProjectFilters}
testModel={testModel}
resolvedConfigFile={queryParams.resolvedConfigFile}
runTests={() => runTests('bounce-if-busy', visibleTestIds)} />
<Toolbar noMinHeight={true}>
{!isRunningTest && !progress && <div className='section-title'>Tests</div>}
Expand Down