ci: fix windows global activation

jpinkney-aws commented 3 months ago

Problem:

Windows ci is no longer working after we moved amazonq tests to the amazonq package

Solution:

Utilize upper case drive letters for getting the root in amazonq/toolkit. That way imports will resolve correctly.

Additional info:

In multi root npm workspaces on windows it looks like imports into other npm workspace packages makes the loaded module id an uppercase drive letter in the node require cache.

E.g. when we import a file from core the module ids inside of amazonq/toolkits node require cache are something like:

C:\${pathToWorkspace}\packages\core\myfile.js

However, internal workspace package imports are lower case drive letters. That means when core imports a module inside of core we see this as:

c:\${pathToWorkspace}\packages\core\myfile.js

This can cause things like globals to be undefined, since tests inside of amazonq/toolkit are looking for upper case module ids, whereas tests inside of core are always looking for lower case module ids (since the tests live inside of core itself)

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

justinmk3 commented 3 months ago

For reference, here's a stacktrace when this happens locally

macOS
can't repro consistently
running core test src/test/shared/errors.test.ts using Extension Tests (current file) for "core"

Activating extension: amazonwebservices.aws-core-vscode
extensionHostProcess.js:147
Extension failed to activate: amazonwebservices.aws-core-vscode: Error: ToolkitGlobals accessed before initialize()
    at Object.get (/…/packages/core/src/shared/extensionGlobals.ts:107:23)
    at new Timeout (/…/packages/core/src/shared/utilities/timeoutUtils.ts:57:35)
    at activateExtension (/…/packages/core/src/shared/utilities/vsCodeUtils.ts:149:48)
    at Runner.<anonymous> (/…/packages/core/src/test/globalSetup.test.ts:55:32)
    at async Mocha._runGlobalFixtures (/…/node_modules/mocha/lib/mocha.js:1214:5)
    at async Mocha.runGlobalSetup (/…/node_modules/mocha/lib/mocha.js:1174:5)
    at async runAsync (/…/node_modules/mocha/lib/mocha.js:1016:11)
extensionHostProcess.js:147
rejected promise not handled within 1 second: Error: ToolkitGlobals accessed before initialize()
extensionHostProcess.js:147
stack trace: Error: ToolkitGlobals accessed before initialize()
    at Object.get (/…/packages/core/src/shared/extensionGlobals.ts:107:23)
    at new Timeout (/…/packages/core/src/shared/utilities/timeoutUtils.ts:57:35)
    at activateExtension (/…/packages/core/src/shared/utilities/vsCodeUtils.ts:149:48)
    at Runner.<anonymous> (/…/packages/core/src/test/globalSetup.test.ts:55:32)
    at async Mocha._runGlobalFixtures (/…/node_modules/mocha/lib/mocha.js:1214:5)
    at async Mocha.runGlobalSetup (/…/node_modules/mocha/lib/mocha.js:1174:5)
    at async runAsync (/…/node_modules/mocha/lib/mocha.js:1016:11)

justinmk3 commented 3 months ago

These failing windows tests are unrelated, can be fixed by special-casing Windows for those tests (or just skip them for now):

  3 failing
  1) runCommand
       shows error
         vscode ISDIR:
     Error: Timed out waiting for message: /EEXIST: file already exists/, displayed:
[
    "Failed to run command: _test.throwable: Error: EPERM: operation not permitted, open 'D:\\'"
]

  2) runCommand
       shows error
         toolkit `PermissionsError`:
     Error: Timed out waiting for message: /incorrect permissions. Expected rw-, found r--/, displayed:
[
    "Failed to run command: _test.throwable: Error: EPERM: operation not permitted, open 'c:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\aws-toolkit-vscode\\vsctka53892c2\\unwritableFile'"
]

  3) runCommand
       shows error
         nodejs EACCES (not wrapped by toolkit `PermissionsError`):
     Error: Timed out waiting for message: /EACCES: permission denied/, displayed:
[
    "Failed to run command: _test.throwable: EPERM: operation not permitted, open 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\aws-toolkit-vscode\\vsctka53892c2\\unwritableFile'"

justinmk3 commented 3 months ago

Is this primarily relevant to the require() call here: https://github.com/aws/aws-toolkit-vscode/blob/6f263c81f58d6d50f5cbc6ae223460a28f6f3146/packages/core/src/test/testRunner.ts#L96-L105

should we add a comment there like see comment in getRoot() ?
maybe also mention NODE_DEBUG=module https://stackoverflow.com/a/21057355/152142
The existing comments there suggest that this may be a mocha issue or at least we may want to (in the future) look more closely at how we're using mocha, since that code basically hasn't change for 5 years :)

Is there any way we can detect this problem to make it less painful in the future? For example, can we iterate node's resolved paths and make a guess about the proper casing? Or can we check the result of require(fullPath) ?

jpinkney-aws commented 3 months ago

should we add a comment there like see comment in getRoot() ?

I'm not sure I understand how these things are related, I think mocha require https://mochajs.org/#-require-module-r-module and what's happening here are two different things. The require.cache from everything coming from aws-core-vscode is already populated before we do anything with mocha

The existing comments there suggest that this may be a mocha issue or at least we may want to (in the future) look more closely at how we're using mocha, since that code basically hasn't change for 5 years :)

It's worth checking to see if we can update this, but I think its a node/vscode issue rather than a mocha issue. I think their requires won't help us here

justinmk3 commented 3 months ago

I'm not sure I understand how these things are related, I think mocha require https://mochajs.org/#-require-module-r-module and what's happening here are two different things.

We are modifying getRoot to fix this issue. Which code using the result of getRoot is affected by this issue? I thought it was the mocha require(), no?

jpinkney-aws commented 3 months ago

We are modifying getRoot to fix this issue. Which code using the result of getRoot is affected by this issue?

Everything that uses getRoot is affected. AFAICT when a new module is loaded all of its imports are put into the require cache depending on the case of their parent modules drive letter. So if globalSetup was required with an upper case drive letter and all the mocha files are loaded with an uppercase drive letter then everything in the require.cache that's loaded inside of toolkit/amazonq will be with an upper case drive letter. Everything in core will always use lower case drive letters though

justinmk3 commented 3 months ago

when a new module is loaded all of its imports are put into the require cache depending on the case of their parent modules drive letter. So if globalSetup was required with an upper case drive letter and all the mocha files are loaded with an uppercase drive letter then everything in the require.cache that's loaded inside of toolkit/amazonq will be with an upper case drive letter. Everything in core will always use lower case drive letters though

This is a much clearer explanation. Could replace the entire comment in getRoot.

aws / aws-toolkit-vscode