@W-19444022@ E2E tests base level #121

kylewalke · 2025-09-02T19:33:01Z

What does this PR do?

Adding E2E tests with Playwright to the repo

What issues does this PR fix or reference?

@W-19444022@

mshanemc

meta: please run npx knip to cleanup unused exports. I don't see a lot of refactoring cleanups or "clean up things while doing another fix/feature", so if we let in a bunch of slop and dead code, it'll be there for a long time.

mshanemc · 2025-09-03T19:12:58Z

.github/workflows/e2e-tests.yml

+
+on:
+  push:
+    branches: [main, kyledev/e2eTesting]


you want to clean this up, right?

Yes, I need it to be there for the duration of the branches existence (this PR) but it would be removed after the fact since this branch will get deleted.

mshanemc · 2025-09-03T19:19:40Z

e2e-tests/README.md

+│   ├── outline-helpers.ts     # Outline view specific helpers
+│   └── global.ts              # Global setup and teardown functions
+├── test-server.js             # VS Code Web test server
+├── playwright.config.ts       # Playwright configuration


I don't think I need this AI slop. Maybe only point out the interesting/unexpected parts?

file/folder names should be doing most of the work.

And trying to maintain it for every file is gonna suck.

Yeah, I can reduce it down to a more general overview and less nitty gritty that will change a lot over time. It's nice to see while working on it but I'm totally ok not immortalizing it.

mshanemc · 2025-09-03T19:29:10Z

packages/apex-ls/src/utils/EnvironmentUtils.ts

-  return typeof globalThis !== 'undefined' && 
-         'indexedDB' in globalThis && 
-         (globalThis as any).indexedDB !== null;
+  return (


is that a real scenario (browser without indexDB?)

Browser no but desktop wouldn't have it, right?

for context, these were all linting auto-fixes I noticed needed auto-fixing while workin on the e2e tests so no functional changes in the apex-ls package files.

mshanemc · 2025-09-03T19:39:11Z

e2e-tests/utils/test-helpers.ts

+
+  if (isVisible) {
+    // Hover to show file selection in debug mode
+    if (process.env.DEBUG_MODE) {


meta: I don't love that there's all these different behaviors depending on the env. Have you found these to be really useful, vs. the tradeoffs of

complexity + extra code

works locally then fails in CI

Like, if we're concerned about parallelism and accuracy, I'd rather run 1-test-per-container in gha than I would run them all sequentially and have that be different from desktop.

Curious on your perspective.

During dev I was having some issues running parallel local headless vs headed mode and with/without the playwright UI, so these env variables were to help nail down exactly what should happen when I run the different scripts. I haven't done as much extensive testing in the CI env to make sure that's all as robust as it probably needs to be (meaning even with what I have here the option2 is the result). I was thinking the tests should be run sequentially instead of parallel because I want to see in order that the tests pass for activating the extension first, then that the server was initialized, then that the file is opened, etc. Since to me, if something goes wrong in that sequential chain, I would then know where to look to fix things. Especially since most of the additional tests are dependent on each other to pass.

I do want CI and desktop to run as similarly as possible though, headless or not, I think sequential execution should be the approach still.

mshanemc · 2025-09-03T19:39:45Z

e2e-tests/utils/test-helpers.ts

+    // Hover to show file selection in debug mode
+    if (process.env.DEBUG_MODE) {
+      await clsFile.hover();
+      await page.waitForTimeout(500);


I'd like to avoid waitForTimeout. It's hard to keep Cursor from doing it, but the tests should be waiting for some screen element, etc.

I can abide by that. It's really easy/convenient but introduces inaccurate cues for when to move on to the next step of a test. I can remove them in place of more precise triggers.

mshanemc · 2025-09-03T19:47:28Z

e2e-tests/utils/global.ts

+  }
+
+  // Create test workspace using fixtures
+  const workspacePath = path.resolve(__dirname, '../test-workspace');


are all the tests going to try to use the same workspace? would it make more sense to have a helper so that a test can set up its workspace (preferably in /tmp) with the files it needs?

Yes. The setup I have right now uses the same directory that is created as the test workspace in the e2e-tests directory so that it's also easy to find. Is there additional benefit to the test workspace and files living in tmp? I think for the base e2e's this is fine but I'd be okay expanding the setup to move the workspace later.

mshanemc · 2025-09-03T19:48:03Z

e2e-tests/utils/global.ts

+
+  // Create test workspace using fixtures
+  const workspacePath = path.resolve(__dirname, '../test-workspace');
+  if (!fs.existsSync(workspacePath)) {


you can just mkdir without checking for existence. That's one of the nice things about recursive

I'm not generally a fan of globalSetup stuff, hopefully it can be removed

I didn't know that about mkdir! nice, okay I can clean that up. Do you mean you don't like setup steps at all or just like the globalSetup naming scheme?

mshanemc · 2025-09-03T19:49:47Z

e2e-tests/utils/global.ts

+ *
+ * @param _config - Playwright configuration
+ */
+export async function globalSetup(_config: FullConfig): Promise<void> {


unused? why have it?

mshanemc · 2025-09-03T19:56:12Z

e2e-tests/fixtures/apex-samples.ts

+/**
+ * Sample Apex class with basic methods for testing language features.
+ */
+export const HELLO_WORLD_CLASS: SampleFile = {


probably 1 file per sample class? other than apex-samples what else would there be in fixtures since this is an Apex LS?

And it's weird to have these separated from the outline-helpers stuff. Like, what if some of these had the EXPECTED_APEX_SYMBOLS so that anything using them could access both, and when you change something here you don't have to change the other file?

Come to think of it, this is a good point. There's not gonna be much else we need to generate so I think this could get merged into the other helpers.

mshanemc · 2025-09-03T19:58:19Z

e2e-tests/fixtures/apex-samples.ts

+/**
+ * Sample Apex trigger for testing trigger-specific functionality.
+ */
+export const ACCOUNT_TRIGGER: SampleFile = {


are you actually using these for anything? I see helloWorld and complex used but not the soql/trigger.

I was thinking I might add some tests to make sure that the different file ending types worked. like .trigger, .cls, and .apex but I didn't flesh out those test cases. This is an artifact from that. I want to get one of those in eventually to make sure the outline and extension activation works for all 3 filetypes still but doesn't need to be part of this initial work.

- Fix Playwright webServer configuration to use correct working directory - Add dynamic cwd detection to run npm scripts from root when executed from e2e-tests directory - Update GitHub Actions workflow to properly handle directory context - Ensure test:e2e:server script can be found and executed correctly This resolves the remaining e2e test failures by ensuring the webServer can properly locate and execute the test server startup script.

- Set headless: true in CI environments to avoid X11 display issues - Add CI-specific Chrome launch arguments for better stability - Keep headed mode for local development debugging - Resolves browser launch failures in GitHub Actions runners This fixes the 'Missing X server or $DISPLAY' error that was preventing the test server from starting in CI environments.

kylewalke added 3 commits August 30, 2025 11:50

feat: e2e testing

d85a16c

feat: expanding to outline verification case

0faf642

fix: more tests for specifically outline view

fcf0e4d

kylewalke requested a review from a team as a code owner September 2, 2025 19:33

kylewalke requested a review from CristiCanizales September 2, 2025 19:33

kylewalke changed the base branch from main to tdx26/main September 2, 2025 19:33

kylewalke added 3 commits September 2, 2025 14:56

fix: removing some cruft

ac2118a

chore: lint fix and cleanup

c6fe687

fix: some structure

83a7f67

kylewalke requested a review from peternhale September 2, 2025 23:48

fix: minor

fbc8c2b

mshanemc reviewed Sep 3, 2025

View reviewed changes

kylewalke added 2 commits September 4, 2025 13:12

fix: pr feedback

5ca3dee

fix: ci fix

d7c3fd0

kylewalke force-pushed the kyledev/e2eTests branch from 40fda3b to 30bee76 Compare September 4, 2025 20:54

fix: improve e2e test CI environment compatibility

e4861f1

kylewalke force-pushed the kyledev/e2eTests branch from 30bee76 to e4861f1 Compare September 4, 2025 20:55

fix: CI fix

27273df

kylewalke force-pushed the kyledev/e2eTests branch from 630bd2a to 27273df Compare September 4, 2025 21:15

kylewalke added 2 commits September 4, 2025 14:20

kylewalke requested a review from mshanemc September 4, 2025 22:37

kylewalke added 4 commits September 5, 2025 11:34

fix: minor config change to fix up for demo

c10fe78

feat: testing CI changes

c5dfdf9

fix: tweak timers

29a6b11

fix: minor CI tweaking still

5b2e9af

@W-19444022@ E2E tests base level #121

Are you sure you want to change the base?

@W-19444022@ E2E tests base level #121

Uh oh!

Conversation

kylewalke commented Sep 2, 2025

What does this PR do?

What issues does this PR fix or reference?

Uh oh!

mshanemc left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

kylewalke Sep 3, 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!

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!

Uh oh!

kylewalke Sep 3, 2025 •

edited

Loading