[BUG] test runner breaks when invoked by path name with the wrong casing

[fkleuver]

Note: this is ultimately a problem between chair and keyboard but I'm filing an issue as it might be worth considering to make Playwright more robust against the problem described here. If not, maybe it'll at least help someone else who runs into this in the future.

Context:

• Playwright Version: 1.15.0
• Operating System: Windows
• Node.js version: 16.10.0 (was also able to repro in 14.17.6 LTS)
• Browser: Chromium / All

Code Snippet

Repo: https://github.com/fkleuver/playwright-test-repro
(although it should be reproducible with any setup)

Simplest steps to repro:

cd C:\projects
git clone [email protected]:fkleuver/playwright-test-repro.git
cd playwright-test-repro
npm ci
npm run --prefix ../Playwright-test-repro e2e
                  # ^ note the capital P 

The above repro steps fail with the following error:

Error: test\w3.spec.js:3:1: test() can only be called in a test file
    at errorWithLocation (C:\projects\Playwright-test-repro\node_modules\@playwright\test\lib\test\util.js:195:10)
    at TestTypeImpl._createTest (C:\projects\Playwright-test-repro\node_modules\@playwright\test\lib\test\testType.js:78:51)
    at C:\projects\Playwright-test-repro\node_modules\@playwright\test\lib\test\transform.js:133:12
    at file:///C:/projects/Playwright-test-repro/test/w3.spec.js:3:1
    at ModuleJob.run (node:internal/modules/esm/module_job:183:25)
    at Loader.import (node:internal/modules/esm/loader:178:24)
    at importModuleDynamicallyWrapper (node:internal/vm/module:437:15)
    at Loader._requireOrImport (C:\projects\playwright-test-repro\node_modules\@playwright\test\lib\test\loader.js:214:52)
    at Loader.loadTestFile (C:\projects\playwright-test-repro\node_modules\@playwright\test\lib\test\loader.js:130:7)
    at Runner._run (C:\projects\playwright-test-repro\node_modules\@playwright\test\lib\test\runner.js:219:40)

The repro steps may seem a bit contrived but it's simply the easiest way to reproduce if you folks want to debug this.

Describe the bug

The "culprit" is module-scoped variables in Playwright which are used as a cache.
File: src/test/globals.ts (fns setCurrentlyLoadingFileSuite / currentlyLoadingFileSuite)

Apparently when the entry point is invoked with the wrong casing, two of those modules end up being loaded. setCurrentlyLoadingFileSuite is called on one instance of the module whereas currentlyLoadingFileSuite is called on another instance (where the backing currentFileSuite variable is always undefined)

How I ran into this: I had a shortcut to open cmd.exe in C:\Projects\my-project (the actual folder is C:\projects\my-project). Typo, my bad, but it worked fine until I switched from mocha to the playwright test runner.

Since the error itself doesn't really help (nor does googling for it), it took a while to find out the root cause.
Imagine the fun:

• run tests: fails
• try a bunch of things, up to eventually cd .. && rmdir my-project /s /q followed by a fresh git clone & install
• run tests: succeeds
• open another command prompt to the same folder, run tests: fails (?!)

I don't know the inner workings and constraints of Playwright well enough to confidently suggest any particular solution, but at least detecting duplicate module instantiation to throw a more helpful error in this scenario would seem like good form.

Something like this at the top of globals.ts would accomplish just that:

const s = Symbol.for('__playwright');
if (globalThis[s]) {
  throw new Error(`more than one instance of globals.js was loaded`);
} else {
  Reflect.defineProperty(globalThis, s, { value: true, configurable: true });
}

Then the error will directly point to this specific issue if it's caused by duplicate module instantiation, rather than potentially send the user on a trail to nowhere :-)

[aslushnikov]

@fkleuver first of all, this is very impressive!

Since the error itself doesn't really help (nor does googling for it), it took a while to find out the root cause.
Imagine the fun:

I feel very sorry for this experience! This seems to be a general node.js issue with case-insensitive file systems: https://stackoverflow.com/questions/23289065/nodejs-require-module-name-case-sensitive-issue

As a result, everything is required two times two times, and the globals.ts suffers especially since it relies upon a global module variable.

Even though we can detect this kind of behavior with your guard, I'd rather not land your workaround since it aims to fix an extreme edge case.

[ttiurani]

Just want to let you know I'm hitting this same problem of double loading of globals.ts, but I don't think this has anything to do with cases in my situation: everything is lower case. My double load happens when I'm trying to call playwright from bazel's rules_nodejs on OSX. Bazel creates a sandbox where it runs the cli and this is probably now some issue with all the symlinks Bazel needs to make different stages of the build process.

In a console.trace() on top of globals.ts, the first run I have:

at Object.<anonymous> (node_modules/@playwright/test/globals.js:26:9)

and the second time it's

at Object.<anonymous> (/private/var/tmp/_bazel_/[cut]/node_modules/@playwright/test/lib/test/globals.js:26:9)

which sounds like maybe some of the symlinks were followed. I'm trying to pinpoint what exactly is going on, but not much luck yet.

p.s. It was very lucky to find this issue, it would have been terribly hard to spot otherwise!

[ttiurani]

Ok, so what happens for me is that the first time globals.ts is loaded is in the very first require's in lib/cli/cli.js, before pretty much anything else, due to it being part of the main imports in the cli itself. When I run the build without symlinks, this is also the only time globals.ts is loaded.

The second time globals.ts gets loaded in my Bazel sandbox is in _requireOrImport:

https://github.com/microsoft/playwright/blob/master/src/test/loader.ts#L198

with either playwright.config.ts or my test file is loaded. This is, I think, because for some reason from '@playwright/test' is now interpreted by node to be something else causing the duplicate.

[fkleuver]

Even though we can detect this kind of behavior with your guard, I'd rather not land your workaround since it aims to fix an extreme edge case.

I'd counter-argue that the less common a bug is, the more time/resources it can cost when it does occur.
From a developer experience point of view, just being an extreme edge case in itself shouldn't be a reason not to account for it unless there is somehow a high technical or maintenance cost to the implementation (which I don't believe there is).

FWIW, duplicate module loading in itself (and the sort of issues that come with it) can also happen when you've got two different versions of the same dependency in your dependency tree.

This issue was reported by users of the Aurelia SPA framework on several occasions when there was a major version bump in one of the lower level deps without the requiring dependency having been updated properly (either by the devs forgetting to update the package.json, by the user forgetting to update the requiring dependency, or by npm not normalizing the package-lock properly). Consequently, webpack (or any other bundler) would end up putting multiple instances of each dep in the tree resulting in similar issues.

I added a similar detection mechanism in v2 for this reason and the clarity of the error has saved quite a few headaches already, as people almost immediately know what to do about it (or at least where to look): https://github.com/aurelia/aurelia/blob/master/packages/metadata/src/index.ts#L1050

In the end it's your call, and I have no more stake in this particular issue since I figured it out, but kindly reconsider for potential future users.

[pavelfeldman]

We have all the input, opening for the votes in order to prioritize.

[asdf23]

ARGHHH!!!!!!!! On windows in bash running the script from /c/myProject/ instead of from /c/MyProject will cause this issue although the test file name is correctly cased.

cd /c/myProject
npx test tests/myTest.spec.ts # Error

cd /c/MyProject
npx test tests/myTest.spec.ts # Working

[z41]

solution for bazel:

use nodejs_binary rule and set entry_point to "node_modules/playwright/cli.js"

What is happening: bazel uses sym links in a weird way, and this forces node.js to think, that runner and tests are referencing different files (js modules). As the result, globals.js is not cached and loaded twice (1 for runner and 1 for tests).

As a side note: both bazel and nodejs have some options to override behavior for symlinks (and module loading), but I didn't find a proper combination of those to solve the issue.

---

You also might need to set chdir for nodejs_binary rule to a proper value. I recommend settings "PW_RUNNER_DEBUG" env variable, when debugging.

[z41]

ttiurani you might be interested, please see my answer above

[jakebanks]

ARGHHH!!!!!!!! On windows in bash running the script from /c/myProject/ instead of from /c/MyProject will cause this issue although the test file name is correctly cased.

cd /c/myProject npx test tests/myTest.spec.ts # Error

cd /c/MyProject npx test tests/myTest.spec.ts # Working

THANK YOU.

Had tests running one afternoon (in Git Bash on Windows), and the next day returned to find beforeEach hook can only be called in a test file.

In this scenario, I find it working from Powershell but not Git Bash.

[mxschmitt]

Seems like this bug only affects PowerShell 5.X and 6.X when using PowerShell 7.X its not possible anymore to enter a directory with incorrect casing. My guess is that PowerShell/PowerShell#9250 was fixing it.

Let me see if we can workaround it.

[pavelfeldman]

Why was this issue closed?

Thank you for your contribution to our project. This issue has been closed due to its limited upvotes and recent activity, and insufficient feedback for us to effectively act upon. Our priority is to focus on bugs that reflect higher user engagement and have actionable feedback, to ensure our bug database stays manageable.

Should you feel this closure was in error, please create a new issue and reference this one. We're open to revisiting it given increased support or additional clarity. Your understanding and cooperation are greatly appreciated.