browser: Get Browser Unit Tests Working (#4255)

* browser: update webpack file for browser

- Makes minor changes to our browser webpack related functionality.
- remove 'fs-extra' dependency

Signed-off-by: nkomonen <nkomonen@amazon.com>

* browser: change how globals is set with globalThis

See documentation for explanation.

Signed-off-by: nkomonen <nkomonen@amazon.com>

* comments + clean up

Signed-off-by: nkomonen <nkomonen@amazon.com>

* minor fixes

Signed-off-by: nkomonen <nkomonen@amazon.com>

---------

Signed-off-by: nkomonen <nkomonen@amazon.com>

Author: nkomonen-amazon

.vscode/launch.json

@@ -31,7 +31,7 @@
             "request": "launch",
             "args": ["--extensionDevelopmentPath=${workspaceFolder}", "--extensionDevelopmentKind=web"],
             "outFiles": ["${workspaceFolder}/dist/**/*.js"],
-            "preLaunchTask": "npm: buildBrowser"
+            "preLaunchTask": "buildBrowserDebug"
         },
         {
             "name": "Extension (webpack)",
@@ -85,14 +85,13 @@
             "debugWebWorkerHost": true,
             "request": "launch",
             "args": [
-                "--disable-extensions",
                 "--extensionDevelopmentPath=${workspaceFolder}",
                 "--extensionDevelopmentKind=web",
                 "--extensionTestsPath=${workspaceFolder}/dist/src/testBrowser/testRunner",
                 "${workspaceRoot}/dist/src/testFixtures/workspaceFolder"
             ],
             "outFiles": ["${workspaceFolder}/dist/src/**/*.js"],
-            "preLaunchTask": "npm: buildBrowser"
+            "preLaunchTask": "buildBrowserDebug"
         },
         {
             "name": "Integration Tests",

.vscode/tasks.json

@@ -37,11 +37,10 @@
             }
         },
         {
-            "label": "npm: buildBrowser",
+            "label": "buildBrowserDebug",
             "type": "npm",
-            "script": "buildBrowser",
-            "dependsOn": ["watch"],
-            "detail": "Builds the webpacked browser bundle"
+            "script": "buildBrowserDebug",
+            "detail": "Builds the webpacked browser bundle for debugging"
         },
         {
             "type": "npm",

docs/browser.md

@@ -78,3 +78,64 @@ to help us visualize the imports and determine which module is importing a certa
       ![Dependency Graph](./images/dependency-graph.svg)
     - Additionally specify a certain dependency with `--reaches` , `npx depcruise src/srcShared/fs.ts --reaches "fs-extra" --output-type dot | dot -T svg > dependency-graph.svg`, to hide unrelated dependencies:
       ![Dependency Graph](./images/dependency-graph-small.svg)
+
+## Global Scoped Objects + Testing behavior
+
+### Summary
+
+-   **When in Browser mode**, state is not shared between the actual extension code and the unit test code. I.e you cannot modify a global variable in the extension code and see that change in the unit tests
+-   State will need to be stored somewhere in `globalThis` to be accessible by tests. Any state not in `globalThis` will not be the same as the actual extension, they are separate.
+
+With the introduction of Browser support, it was discovered that the context between the extension code and test code is not shared.
+Though it is shared in the Node version of the extension.
+
+Example that does NOT work in the Browser:
+
+```typescript
+export let myGlobal = 'A'
+
+function activate() {
+    // Change the global variable value
+    myGlobal = 'B'
+}
+```
+
+```typescript
+// Browser unit test
+import { myGlobal } from '../src/extension.ts'
+
+describe('test', function () {
+    it('test', function () {
+        assert.strictEqual(myGlobal, 'B') // this fails in Browser (but not Node.js). The value here is actually 'A'.
+    })
+})
+```
+
+Example that DOES work in the Browser:
+
+```typescript
+;(globalThis as any).myGlobal = 'A'
+
+function activate() {
+    ;(globalThis as any).myGlobal = 'B'
+}
+```
+
+```typescript
+// Browser unit test
+describe('test', function () {
+    it('test', function () {
+        assert.strictEqual((globalThis as any).myGlobal, 'B') // this succeeds in Browser and Node.js
+    })
+})
+```
+
+### Web Worker
+
+The assumption for the behavior is due to how Web Workers work. We (VS Code) use them to run our extension and unit test code when in the Browser. The scripts share global values differently compared to a different environment such as Node.js.
+
+-   [`WorkerGlobalScope`](https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope)
+    -   The context of the executing code is contained within itself and is not accessible to other scripts (eg: extension code context is not accessible by unit tests)
+    -   VS Code uses Dedicated Workers since `globalThis` is indicated as a [`DedicatedWorkerGlobalScope`](https://developer.mozilla.org/en-US/docs/Web/API/DedicatedWorkerGlobalScope) when debugging
+    -   `globalThis` is one object (that I could find so far) which **is shared** between our extension and test scripts. A guess to why is that the main script spawns another web worker (for unit tests) and passes on the `DedicatedWorkerGlobalScope`. See [`"Workers can also spawn other workers"`](https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Functions_and_classes_available_to_workers).
+    -   `globalThis` returns `global` in Node.js, or a `WorkerGlobalScope` in the browser

package.json

@@ -4238,8 +4238,8 @@
         "reset": "npm run clean -- node_modules && npm install",
         "copyFiles": "ts-node ./scripts/build/copyFiles.ts",
         "buildScripts": "npm run generateClients && npm run generatePackage && npm run generateNonCodeFiles && npm run copyFiles",
-        "buildBrowser": "npm run clean && npm run buildScripts && webpack --config webpack.browser.config.js --mode production && npm run copyFiles",
-        "runInBrowser": "npm run buildBrowser && npx @vscode/test-web --open-devtools --extensionDevelopmentPath=. .",
+        "buildBrowserDebug": "npm run clean && npm run buildScripts && webpack --config webpack.browser.config.js",
+        "runInBrowser": "npm run buildBrowserDebug && npx @vscode/test-web --open-devtools --extensionDevelopmentPath=. .",
         "compile": "npm run clean && npm run buildScripts && webpack --mode development && npm run copyFiles",
         "watch": "npm run clean && npm run buildScripts && tsc -watch -p ./",
         "postinstall": "npm run generateTelemetry && npm run generateConfigurationAttributes && npm run buildCustomLintPlugin",

src/common/browserUtils.ts

@@ -4,16 +4,16 @@
  */
 
 import * as vscode from 'vscode'
+import globals from '../shared/extensionGlobals'
 
-let inBrowser = false
 /** Set the value of if we are in the browser. Impacts {@link isInBrowser}. */
 export function setInBrowser(value: boolean) {
-    inBrowser = value
+    globals.isInBrowser = value
     vscode.commands.executeCommand('setContext', 'aws.isWebExtHost', true).then(undefined, e => {
         console.error('setContext failed: %s', (e as Error).message)
     })
 }
 /** Return true if we are running in the browser, false otherwise. */
 export function isInBrowser() {
-    return inBrowser
+    return (globals.isInBrowser ??= false)
 }

src/shared/extensionGlobals.ts

@@ -75,7 +75,27 @@ function getBrowserAlternatives() {
     return alternatives
 }
 
-const globals = { clock: copyClock() } as ToolkitGlobals
+/**
+ * Why was the following implemented this way?
+ *
+ * With the introduction of browser support + setting up browser unit tests,
+ * it was discovered that variables declared at the global scope were not available
+ * between the actual extension code and the unit tests. Though for the regular desktop/node
+ * tests, global scope variables WERE shared and the following was not required.
+ *
+ * So as a solution, any global scoped objects must be stored in `globalThis` so that if defined
+ * in Browser-compatible extension code, it shares the same context/scope as the Browser unit test code.
+ *
+ * See `browser.md` for more info.
+ */
+function resolveGlobalsObject() {
+    if ((globalThis as any).globals === undefined) {
+        ;(globalThis as any).globals = { clock: copyClock() } as ToolkitGlobals
+    }
+    return (globalThis as any).globals
+}
+
+const globals: ToolkitGlobals = resolveGlobalsObject()
 
 export function checkDidReload(context: ExtensionContext): boolean {
     return !!context.globalState.get<string>('ACTIVATION_LAUNCH_PATH_KEY')
@@ -145,4 +165,6 @@ interface ToolkitGlobals {
         endpoints: string
         lambdaSampleRequests: string
     }
+    /** If this extension is running in the Browser (webworker), compared to running on the desktop (node) */
+    isInBrowser: boolean
 }

src/testBrowser/temp.test.ts

@@ -3,16 +3,19 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+/**
+ * The following was influenced by this guide: https://code.visualstudio.com/api/extension-guides/web-extensions
+ */
+
 import 'mocha' // Imports mocha for the browser, defining the `mocha` global.
-import { activate as activateBrowserExtension } from '../extensionWeb'
-import vscode from 'vscode'
+import * as vscode from 'vscode'
+import { VSCODE_EXTENSION_ID } from '../shared/extensions'
 
-export function run(): Promise<void> {
+export async function run(): Promise<void> {
+    await activateToolkitExtension()
     return new Promise(async (resolve, reject) => {
         setupMocha()
         gatherTestFiles()
-        stubVscodeWindow()
-        await runBrowserExtensionActivation()
 
         try {
             runMochaTests(resolve, reject)
@@ -36,23 +39,14 @@ function gatherTestFiles() {
     importAll(require.context('.', true, /\.test$/))
 }
 
-function stubVscodeWindow() {
-    // We skip this for now since getTestWindow() has transitive imports
-    // that import 'fs' and cause issues.
-    // TODO: Go through the transitive imports and swap the 'fs' uses
-    // with our own browser compatible ones.
-    // globalSandbox.replace(vscode, 'window', getTestWindow())
-}
-
 /**
- * This is the root function that activates the toolkit extension in a browser
- * context.
+ * Typically extensions activate depending on their configuration in `package.json#activationEvents`, but in tests
+ * there is a race condition for when the extension has finished activating and when we start the tests.
+ *
+ * So this function ensures the extension has fully activated.
  */
-async function runBrowserExtensionActivation() {
-    await activateBrowserExtension({
-        logUri: vscode.Uri.parse('./tempLogFile.txt'),
-        subscriptions: [],
-    } as any)
+async function activateToolkitExtension() {
+    await vscode.extensions.getExtension(VSCODE_EXTENSION_ID.awstoolkit)?.activate()
 }
 
 function runMochaTests(resolve: (value: void | PromiseLike<void>) => void, reject: (reason?: any) => void) {

src/testBrowser/testRunner.ts

@@ -8,6 +8,7 @@ import { isInBrowser } from '../common/browserUtils'
 
 describe('isInBrowser', function () {
     it('returns true when in browser', function () {
+        // Note that this only works since the state is indirectly stored in `globalThis`, see browser.md for more info
         assert.strictEqual(isInBrowser(), true)
     })
 })

src/testBrowser/utils.test.ts

@@ -32,7 +32,6 @@ const baseConfig = {
         path: path.resolve(__dirname, 'dist'),
         filename: '[name].js',
         libraryTarget: 'commonjs2',
-        devtoolModuleFilenameTemplate: '../[resource-path]',
     },
     devtool: 'source-map',
     externals: {