Skip to content

Commit

Permalink
fix(bazel): integration rule using incorrect casing for working dir o…
Browse files Browse the repository at this point in the history
…f commands

The integration rule currently directly takes the Bazel `TEST_TMP_DIR`
as directory for the integration test. Bazel will use a lower-case
variant of the path on case-insensitive platforms (like Windows). This
is acceptable for standard IO operations (as within Bazel), but can
unexpectedly break module resolution/or cause subtle differences.

We should always normalize the path to an actual case-exact system
realpath when setting it as `CWD` for command invocations. This
enables the use of e.g. playwright within Bazel integration tests.

microsoft/playwright#9193.
  • Loading branch information
devversion committed Nov 12, 2021
1 parent 0ef299c commit afceae0
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 3 deletions.
1 change: 1 addition & 0 deletions bazel/integration/test_runner/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ts_library(
"@npm//@types/node",
"@npm//@types/tmp",
"@npm//tmp",
"@npm//true-case-path",
],
)

Expand Down
11 changes: 11 additions & 0 deletions bazel/integration/test_runner/file_system_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import * as fs from 'fs';
import {trueCasePath} from 'true-case-path';

/** Gets whether the file is executable or not. */
export async function isExecutable(filePath: string): Promise<boolean> {
Expand All @@ -18,6 +19,16 @@ export async function isExecutable(filePath: string): Promise<boolean> {
}
}

/**
* Gets a case-exact system realpath for the specified path.
*
* This is useful for example because Bazel passes `C:\users\<..>` as action input, but
* the actual case-exact path for the current platform would be: `C:\Users\<..>`.
*/
export async function getCaseExactRealpath(filePath: string): Promise<string> {
return trueCasePath(filePath);
}

/** Adds the `write` permission to the given file using `chmod`. */
export async function addWritePermissionFlag(filePath: string) {
if (await isExecutable(filePath)) {
Expand Down
12 changes: 9 additions & 3 deletions bazel/integration/test_runner/process_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import * as childProcess from 'child_process';
import * as path from 'path';
import {debug} from './debug';
import {getCaseExactRealpath} from './file_system_utils';

/**
* Regular expression matching environment variable substitutions
Expand Down Expand Up @@ -41,21 +42,26 @@ export function expandEnvironmentVariableSubstitutions(
* @returns a Promise that resolves with a boolean indicating whether the
* command completed successfully or not.
*/
export function runCommandInChildProcess(
export async function runCommandInChildProcess(
binary: string,
args: string[],
workingDir: string,
env: NodeJS.ProcessEnv,
): Promise<boolean> {
const humanReadableCommand = `${binary}${args.length ? ` ${args.join(' ')}` : ''}`;
// Note: We resolve the working directory to a case-exact system `realpath`. This is
// necessary as otherwise Node module resolution could behave unexpectedly when invoked
// tools down-the-line resolve files with an actual system realpath. Here is an example
// within Microsoft's `playwright`: https://github.com/microsoft/playwright/issues/9193.
const normalizedWorkingDir = await getCaseExactRealpath(path.posix.normalize(workingDir));

debug(`Executing command: ${humanReadableCommand} in ${workingDir}`);
debug(`Executing command: ${humanReadableCommand} in ${normalizedWorkingDir}`);

return new Promise<boolean>((resolve) => {
const commandProcess = childProcess.spawn(binary, args, {
shell: true,
stdio: 'inherit',
cwd: workingDir,
cwd: normalizedWorkingDir,
env,
});

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"selenium-webdriver": "3.5.0",
"semver": "^7.3.5",
"tmp": "^0.2.1",
"true-case-path": "^2.2.1",
"ts-node": "^10.2.1",
"tslib": "^2.3.0",
"tslint": "^6.1.3",
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3091,6 +3091,11 @@ trim-newlines@^3.0.0:
resolved "https://registry.yarnpkg.com/trim-newlines/-/trim-newlines-3.0.1.tgz#260a5d962d8b752425b32f3a7db0dcacd176c144"
integrity sha512-c1PTsA3tYrIsLGkJkzHF+w9F2EyxfXGo4UyJc4pFL++FMjnq0HJS69T3M7d//gKrFKwy429bouPescbjecU+Zw==

"true-case-path@^2.2.1":
version "2.2.1"
resolved "https://registry.yarnpkg.com/true-case-path/-/true-case-path-2.2.1.tgz#c5bf04a5bbec3fd118be4084461b3a27c4d796bf"
integrity sha512-0z3j8R7MCjy10kc/g+qg7Ln3alJTodw9aDuVWZa3uiWqfuBMKeAeP2ocWcxoyM3D73yz3Jt/Pu4qPr4wHSdB/Q==

ts-node@^10.2.1:
version "10.4.0"
resolved "https://registry.yarnpkg.com/ts-node/-/ts-node-10.4.0.tgz#680f88945885f4e6cf450e7f0d6223dd404895f7"
Expand Down

0 comments on commit afceae0

Please sign in to comment.