test: VSCode test setup and switch to insiders VSCODE-700 #1186

tculig · 2025-11-16T22:24:20Z

Description

The problem was: The vscode test setup is failing with both stable and insiders builds and that is because the recent changes from VSCode does not allow having multiple providers for a registered scheme.

Several people attempted a solution, and I got feedback to try to remove the MDBExtensionController stub, and instead use the actual controller that is initialized in the src/extension.ts file. This seemed like a good solution, but it caused more problems than it solved:

using real controller during unit tests caused links to be opened in browser, files to be opened and saved, and an unknown quantity of other test actions to be performed for real
Overriding any of the fields or functions from the MDBExtensionController seems to be impossible once it gets registered. Probably there is a hacky way to do it, but they definitely do not want this done.
A number of tests started failing when run through VSCode UI, which pass when running through CLI, so some context collision is still happening

After doing a thorough exploration in mitigation of above mentioned problems, I concluded that this cannot be the way forward. Aside from everything in the system working against this solution, it would also require an update to many of the current unit tests (about 60 at a minimum).

I though about opening a ticket directly on VSCode github page, but with over 13k open issues, I figured it might not be the optimal solution. Googling for the exact error I was getting when using insiders build returned no exact matches, and tracking down even a StackOverflow thread with the similar issue proved difficult.

For all these reasons I concluded that the solution is either extremely complex, or incredibly trivial.
I went for incredibly trivial, and this PR is that solution.
Using a env flag prevents the real controller from being registered when running tests, and registers ONLY the stubbed one instead. It seems to resolve the issue, and the unit tests run normally.
There were some issues when running via the VSCode UI, which pertain to specific test cases, so I had to update those as well.
It was all down to multiple registration/initializations, incomplete test cleanup which caused the first run to be successful, but the subsequent ones to fail, and few other minor things.

In summation, we can now run test from CLI or VSCode UI, the github actions are all working. The version we test against is now "insiders" again.

Checklist

New tests and/or benchmarks are included
Documentation is changed or added
I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

Bugfix
New feature
Dependency update
Misc

Open Questions

Dependents

Types of changes

Backport Needed
Patch (non-breaking change which fixes an issue)
Minor (non-breaking change which adds functionality)
Major (fix or feature that would cause existing functionality to change)

himanshusinghs · 2025-11-17T10:16:39Z

src/extension.ts

-let mdbExtension: MDBExtensionController;
+let mdbExtension: MDBExtensionController | undefined;
+
+const isUnderTest = process.env.MDB_UNDER_TEST === 'true';


Suggested change

const isUnderTest = process.env.MDB_UNDER_TEST === 'true';

const isUnderTest = process.env.NODE_ENV === "test" && process.env.MDB_UNDER_TEST === 'true';

nit and optional: Perhaps to make it more clear that this is a test only setting. Doing this would require setting the correct flag when running tests as well.

maybe let's get rid of MDB_UNDER_TEST altogether and just use the NODE_ENV?

Could be problematic

I prefer being explicit about this if you're not married to the idea of using NODE_ENV @gagik

okay NODE_DEV in particular actually antipattern (TIL), although not necessarily relevant for our case: https://nodejs.org/en/learn/getting-started/nodejs-the-difference-between-development-and-production

"under test" is weird wording for me though, maybe just MDB_IS_TEST?

Would that create problems for us when/if we migrate to using vitest, considering it doesn't automatically set NODE_ENV? Or do we just set it ourselves?

we'd set it just like any other environment variable yeah

For the current setup, it actually doesn't set the NODE_ENV to "test" automatically, just FYI

yeah it's just a matter of naming preferences. The real difference here is that in production environment this is pre-set to production so the variable is less random.
NODE_ENV === 'test' or MDB_IS_TEST is fine by me; but it should be one or the other imo

So, as it turns out, NODE_ENV is being set to "development" by webpack. It overrides any value that is being passed as a runtime parameter. Thus, it is somewhat unclear when this variable will get hijacked by some other process, and is definitely not 100% under our full control. I would argue for keeping MDB_IS_TEST (renamed from MDB_UNDER_TEST), for consistency and full control @gagik

Co-authored-by: Himanshu Singh <himanshu.singhs@outlook.in>

himanshusinghs

Looking good 🚀

himanshusinghs · 2025-11-17T15:40:55Z

src/test/runTest.ts

  // Download VS Code, unzip it and run the integration test
  await runTests({
-    version: '1.103.2', // TODO(VSCODE-700) Once we fix the test setup issues, we should revert this to 'insiders'
+    version: 'insiders',


himanshusinghs · 2025-11-17T15:42:24Z

Feel free to merge whenever you've addressed the open comment.

gagik

Haven't looked at rest too thoroughly but MDB_IS_TEST sgtm!

fix running test in VSCode UI

ab321dd

tculig force-pushed the VSCODE-700-fix-VSCode-test-setup-and-switch-to-insiders branch from 66057bb to ab321dd Compare November 16, 2025 22:29

Update github actions

913f5b2

tculig marked this pull request as ready for review November 17, 2025 09:55

tculig requested a review from a team as a code owner November 17, 2025 09:55

himanshusinghs reviewed Nov 17, 2025

View reviewed changes

tculig and others added 2 commits November 17, 2025 11:32

Add note explaining the early return

621c801

Update src/extension.ts

b592233

Co-authored-by: Himanshu Singh <himanshu.singhs@outlook.in>

tculig changed the title ~~Fix VSCode test setup and switch to insiders VSCODE-700~~ text: VSCode test setup and switch to insiders VSCODE-700 Nov 17, 2025

tculig changed the title ~~text: VSCode test setup and switch to insiders VSCODE-700~~ test: VSCode test setup and switch to insiders VSCODE-700 Nov 17, 2025

Add MDB_UNDER_TEST flag to test-extension script

07f26cd

tculig force-pushed the VSCODE-700-fix-VSCode-test-setup-and-switch-to-insiders branch from a17d2b4 to 07f26cd Compare November 17, 2025 10:53

himanshusinghs approved these changes Nov 17, 2025

View reviewed changes

tculig added 2 commits November 19, 2025 14:18

Rename MDB_UNDER_TEST to MDB_IS_TEST

e9f58ec

Remove NODE_ENV check

104833d

tculig requested a review from gagik November 19, 2025 13:30

gagik approved these changes Nov 19, 2025

View reviewed changes

tculig merged commit f45a4c7 into main Nov 20, 2025
12 checks passed

tculig deleted the VSCODE-700-fix-VSCode-test-setup-and-switch-to-insiders branch November 20, 2025 08:49

	const isUnderTest = process.env.MDB_UNDER_TEST === 'true';
	const isUnderTest = process.env.NODE_ENV === "test" && process.env.MDB_UNDER_TEST === 'true';

test: VSCode test setup and switch to insiders VSCODE-700 #1186

test: VSCode test setup and switch to insiders VSCODE-700 #1186

Uh oh!

Conversation

tculig commented Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Motivation and Context

Open Questions

Dependents

Types of changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tculig Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

himanshusinghs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

himanshusinghs commented Nov 17, 2025

Uh oh!

gagik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tculig commented Nov 16, 2025 •

edited

Loading

tculig Nov 19, 2025 •

edited

Loading