microsoft / vscode-test-cli

Command-line runner for VS Code tests
MIT License
20 stars 8 forks source link

Mocha drive letter case sensitivity bug #49

Closed lovettchris closed 2 months ago

lovettchris commented 2 years ago

I found a mocha bug that is fixed with the following hack, so you might want to add this to your sample index.ts files:

export function run(testsRoot: string, cb: (error: any, failures?: number) => void): void {
    // Create the mocha test
    const mocha = new Mocha({
        ui: 'tdd'
    });

    if (process.platform === 'win32') {
        // workaround for https://github.com/microsoft/vscode-test/issues/134
        // see also https://github.com/mochajs/mocha/issues/4851
        testsRoot = testsRoot.toLowerCase();
    }
connor4312 commented 2 years ago

Casing bugs are always lovely 😆

Is this the same as https://github.com/microsoft/vscode-test/issues/134?

lovettchris commented 2 years ago

This bug is about making your tests work until the mocha bug is fixed, so yes, they are related.

eliericha commented 5 months ago

Hello folks. I'm running into a similar issue, but I'm not sure how I can implement a workaround.

I'm using a .vscode-test.mjs file to define test configurations. It looks something like this:

        return {
            ...
            files: `out/test/suite/gnattest/**/*.test.js`,
            ...
        }

When I run vscode-test without arguments, I get failures like this:

     TypeError: Cannot read properties of undefined (reading 'getProjectFile')
        at Context.<anonymous> (C:\data\ancr\src\als\integration\vscode\ada\out\test\suite\gnattest\gnattest.test.js:16:70)

Basically a piece of code in the test is reading a global variable exported by a module from the extension under test, and this global variable is unexpectedly undefined.

Some more digging showed that callingvscode-test with a --run argument fixes the issue, something like this:

vscode-test --run c:\data\ancr\src\als\integration\vscode\ada\out\test\suite\gnattest\gnattest.test.js

Note how it's c: while we see C: in the above backtrace. So now if I try a capital C::

vscode-test --run C:\data\ancr\src\als\integration\vscode\ada\out\test\suite\gnattest\gnattest.test.js

It fails again. That's how I figured out it may be related to this issue.

However in my case I don't have an index.ts file with a run function so I don't have a place to normalize the casing. And I don't want to create one because I want my setup to work with the Extension Test Runner which is why I'm using a .vscode-test.mjs configuration file.

So now my issue is: how do I run all tests? That's typically done by simply calling vscode-test and that runs into the path casing bug.

Any ideas?

Thanks!

connor4312 commented 5 months ago

Are you able to share more info so I can try to reproduce the problem you're seeing?

eliericha commented 5 months ago

Let me see if I can reproduce the problem with the sample CLI project

eliericha commented 5 months ago

I managed to reproduce on a fork of the sample test CLI project

First I got the following problem when running from the VS Code Testing view:

 >  vscode-test --label 0 --reporter c:\data\ancr\src\vscode-extension-samples\helloworld-test-cli-sample\node_modules\@vscode\test-cli\out\fullJsonStreamReporter.cjs --run c:\data\ancr\src\vscode-extension-samples\helloworld-test-cli-sample\out\test\suite\extension.test.js --grep /^(Extension Test Suite Sample test$)/
Downloading VS Code 1.87.2 from https://update.code.visualstudio.com/1.87.2/win32-x64-archive/stable
Downloading VS Code (135878169B)
gzip: stdin has more than one entry--rest ignored
node:events:492
      throw er; // Unhandled 'error' event
      ^

Error: write EPIPE
    at afterWriteDispatched (node:internal/stream_base_commons:160:15)
    at writeGeneric (node:internal/stream_base_commons:151:3)
    at Socket._writeGeneric (node:net:931:11)
    at Socket._write (node:net:943:8)
    at doWrite (node:internal/streams/writable:411:12)
    at clearBuffer (node:internal/streams/writable:572:7)
    at onwrite (node:internal/streams/writable:464:7)
    at WriteWrap.onWriteComplete [as oncomplete] (node:internal/stream_base_commons:106:10)
Emitted 'error' event on Socket instance at:
    at Socket.onerror (node:internal/streams/readable:785:14)
    at Socket.emit (node:events:514:28)
    at emitErrorNT (node:internal/streams/destroy:151:8)
    at emitErrorCloseNT (node:internal/streams/destroy:116:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -4047,
  code: 'EPIPE',
  syscall: 'write'
}

Node.js v18.17.1
Error: Test process exited with code 1

However I'd already seen this, but can't find the relevant GitHub issue now. A npm upgrade got me past it.

Next I updated the test to make use of global variables exposed by the extension module. The test passed in when launched from the Testing view of VS Code because that uses --run c:\... with a lowercase c:.

However when I try launching from a PowerShell in that directory, I get this:

PS C:\data\ancr\src\vscode-extension-samples\helloworld-test-cli-sample> npm test

> helloworld-sample@0.0.1 test
> vscode-test

Found existing install in C:\data\ancr\src\vscode-extension-samples\helloworld-test-cli-sample\.vscode-test\vscode-win32-x64-archive-1.87.2. Skipping download

[main 2024-03-26T17:27:37.968Z] update#setState disabled
[main 2024-03-26T17:27:37.968Z] update#ctor - updates are disabled by the environment
Started local extension host with pid 2852.
Loading development extension at c:\data\ancr\src\vscode-extension-samples\helloworld-test-cli-sample

  Extension Test Suite
Congratulations, your extension "helloworld-sample" is now active!
    1) Sample test
  0 passing (104ms)
  1 failing
  1) Extension Test Suite
       Sample test:
     AssertionError [ERR_ASSERTION]: undefined == 'Some value'
        at Context.<anonymous> (C:\data\ancr\src\vscode-extension-samples\helloworld-test-cli-sample\out\test\suite\extension.test.js:17:16)

1 test failed.
[main 2024-03-26T17:27:39.601Z] Extension host with pid 2852 exited with code: 0, signal: unknown.
Exit code:   1
Failed

Note that I've had to remove the pretest script because otherwise I was getting these compilation erros:

PS C:\data\ancr\src\vscode-extension-samples\helloworld-test-cli-sample> npm run compile

> helloworld-sample@0.0.1 compile
> tsc -p ./

node_modules/@types/glob/index.d.ts:29:42 - error TS2694: Namespace '"C:/data/ancr/src/vscode-extension-samples/helloworld-test-cli-sample/node_modules/minimatch/dist/cjs/index"' has no exported member 'IOptions'.

29     interface IOptions extends minimatch.IOptions {
                                            ~~~~~~~~

node_modules/@types/glob/index.d.ts:74:30 - error TS2724: '"C:/data/ancr/src/vscode-extension-samples/helloworld-test-cli-sample/node_modules/minimatch/dist/cjs/index"' has no exported member named 'IMinimatch'. Did you mean 'Minimatch'?

74         minimatch: minimatch.IMinimatch;
                                ~~~~~~~~~~

Found 2 errors in the same file, starting at: node_modules/@types/glob/index.d.ts:29

Let me know if you need more information. I'd be interested in seeing if you can reproduce the issue!

Thanks

eliericha commented 5 months ago

Oh and here are my OS specs if that's relevant

Edition Windows Server 2022 Datacenter Version 21H2 Installed on ‎3/‎26/‎2024 OS build 20348.2322

And

PS C:\data\ancr\src\vscode-extension-samples\helloworld-test-cli-sample> node -v
v18.15.0
connor4312 commented 5 months ago

Hm, I'm not able to repro this. Here's what I'm trying, is that different that what you're doing? https://github.com/microsoft/vscode-extension-samples/tree/connor412/try-repro/helloworld-test-cli-sample

eliericha commented 5 months ago

I may have used the wrong terminology as I am new to JS. Apologies.

I talked about a global variable when I should be talking about a module export. globalThis like you tried works. But module exports don't, at least in my setup. Here's what I'm doing:

https://github.com/eliericha/vscode-extension-samples/commit/02937bd1197a4e3b8336d4a95b862f38354fdbdc

connor4312 commented 5 months ago

It seems like if the upper-cased drive letter is used we end up importing the module twice, which is evident when adding a top-level console.log to extension.ts

did import C:\Users\conno\Github\vscode-extension-samples\helloworld-test-cli-sample\out\extension.js
did import c:\Users\conno\Github\vscode-extension-samples\helloworld-test-cli-sample\out\extension.js

@alexdima this seems like something the loader should handle, or should we do some treatment for the extension tests URI first?

https://github.com/microsoft/vscode/blob/80a9d241489b53d4ed99198a6f85408b0cfe5ab9/src/vs/workbench/api/common/extHostExtensionService.ts#L743

eliericha commented 5 months ago

Pending a fix, is there a workaround that would allow me to run vscode-test from a shell or a CI runner?

The only thing I've seen work is passing --run. So perhaps I could wrap the vscode-test call in a script that collect the files corresponding to tests that should be executed.

Can you think of something more practical?

eliericha commented 5 months ago

After some fiddling around, I was able to find a simpler workaround which is to spawn the node process in a way that makes the current work directory use a lowercase drive letter.

I managed to do that with the following Python script:

from pathlib import Path
import subprocess

cwd = Path().absolute()

# Convert the path to a lowercase drive letter
cwd = Path(f"{cwd.drive.lower()}\\", *cwd.parts[1:])

cmd = ["npm", "run", "test"]
exit(subprocess.call(cmd, cwd=cwd, shell=True))
alexdima commented 3 months ago

Here is where we load the extension test entry file https://github.com/microsoft/vscode/blob/b7e5750fa94dabc99f2ee4f48f73de7a025a9d64/src/vs/workbench/api/common/extHostExtensionService.ts#L743

and here is where we load the extension https://github.com/microsoft/vscode/blob/b7e5750fa94dabc99f2ee4f48f73de7a025a9d64/src/vs/workbench/api/common/extHostExtensionService.ts#L487

In both cases we end up calling _loadCommonJSModule, passing in the URI.

If the URIs have different casings, it means they were "massaged" somewhere on the way to the extension service, but I can't immediately say who would massage the drive letter casing. Any thoughts @jrieken

connor4312 commented 2 months ago

It turns out the Node.js CJS loader is also case sensitive, never knew that! So I think I should just make a change in vscode-test-cli to normalize the casing of imported modules.

eliericha commented 2 months ago

Hello folks,

Is there a release of @vscode/test-cli planned soon to benefit from this fix?

Thanks

connor4312 commented 2 months ago

0.0.10 was released with this fix

eliericha commented 2 months ago

I see that the Extension Test Runner version 0.0.10 has been released, but I don't see a release of the @vscode/test-cli npm package. Will that also be released any time soon?

Thanks

connor4312 commented 2 months ago

Ah, sorry, forgot to trigger the build for that. Should be out in a few minutes.