chore(e2e): restructure e2e test helpers to better separate shared state and setup / teardown functions COMPASS-8361 #6383

gribnoysup · 2024-10-21T11:08:25Z

This is a pre-requisite work I'm doing as part of adding e2e tests for logged in Atlas user in compass-web sandbox. We're starting to have a decent amount of branching logic in setup and teardown based on what exact type of e2e we are running: desktop packaged / just compiled, compass-web, compass-web with atlas login. Most of these will run exactly the same suites, but managing the initial setup logic starts to get a bit hairy, so this patch does some refactoring to try to address that:

Normalized "default" connections state to be derived from a single variable instead of manually deriving from multiple ones. This should allow to easier control this in environments like atlas cloud compass-web where the data for those is more dynamic and we don't have local clusters at all
Moved all the "context" that the tests operate on to a single file that re-exports all these consts we have based on various flags in environment and runtime
Moved the setup and teardown to the global-fixtures file following mocha docs
Changed the clean-up logic to rely on mocha for running the required hooks, it now takes a bit more time to stop the tests, but it also seems to leave less things behind when exiting
Some drive-by cleanup:
- We weren't looking at the uploaded results for awile after configuring reporting in evergreen directly, removed that code
- Changed how electron chromium version detection is done so that we don't need extra separate scripts to run

…ate and setup / teardown functions

gribnoysup · 2024-10-21T11:09:47Z

packages/compass-e2e-tests/helpers/compass.ts

-
 // should we test compass-web (true) or compass electron (false)?
-export const TEST_COMPASS_WEB = process.argv.includes('--test-compass-web');
+export const TEST_COMPASS_WEB = _TEST_COMPASS_WEB;


I'll do a follow-up to change the exports for all re-exported vars, just didn't want to make this patch too noisy

gribnoysup · 2024-10-21T11:14:26Z

packages/compass-e2e-tests/helpers/test-runner-global-fixtures.ts

+
+  if (TEST_COMPASS_DESKTOP_PACKAGED_APP) {
+    debug('Building Compass before running the tests ...');
+    await buildCompass();


Some of these probably can be just moved here directly as they are just running scripts with crossSpawn, then it's maybe easier to read through the compass.ts file

lerouxb · 2024-10-21T12:51:24Z

packages/compass-e2e-tests/package.json

    "debug": "^4.3.4",
    "depcheck": "^1.4.1",
    "electron": "^30.5.1",
+    "electron-to-chromium": "^1.5.41",


nice! Would be worried about what happens if we're ever ahead of them, but I suppose that's unlikely because I see this package is a dep of browserslist so should be safe enough.

Yeah, that's a valid point, generally speaking you're right, we'd probably always stay on the same version without any extra effort from our side, but I'm also doing something similar in this other WIP PR, and will copy this part that makes sure that we update electron with browserslist in this one

lerouxb · 2024-10-21T12:54:10Z

packages/compass-e2e-tests/helpers/compass.ts

  semverCondition: string,
  enterpriseExact?: boolean
 ) => {
+  const { version, enterprise } = DEFAULT_CONNECTIONS_SERVER_INFO[0];


Probably not for this PR and I realise you found it this way, but should we continue to assume the connections are all running the same server version?

Good question! Restructuring this code a bit just highlighted this assumption we're making that's technically is not very safe, but at the same time the second connection is almost always created just so we have multiple running at the same time and so the assumption we're making feels safe? We probably can make it all more explicit somehow, at least documenting it for the method as the easiest step 🙂

I think we can do a few things here if we wanted to, one thing that comes to mind right away is that serverSatisfies can take a connection name as an argument, so that we're explicit against what we're checking here. I am planning to do a follow-up specifically to align a bit the skipIf / satisfies interfaces, so might be a good point to do something like this.

Yes it feels safe and indeed only became obvious because of the restructure. We indeed only have multiple connections since recently just for the sake of having two for the MC feature. Only thing I can think of is that someone one day might be running servers of multiple different versions at once, but then this will quickly become an obvious bug they will find.

Just thinking that we could start multiple servers of different versions then for the handful of cases where we do care about different results for different versions we could wrap the test in a for loop. Then we probably don't ever have to run the entire e2e test suite multiple times for different server versions in CI which could alleviate some of the limited mac resource problems..

Anyway. That's just me thinking out loud. I don't think we need to do anything right now. And there might be an opportunity here in future :)

lerouxb · 2024-10-21T12:56:10Z

packages/compass-e2e-tests/helpers/compass.ts

      // eslint-disable-next-line @typescript-eslint/no-var-requires
      const { ipcRenderer } = require('electron');
-      ipcRenderer.invoke('compass:maximize');
+      void ipcRenderer.invoke('compass:maximize');


oops. that was me.

lerouxb · 2024-10-21T12:58:42Z

packages/compass-e2e-tests/helpers/test-runner-context.ts

+  );
+}
+
+// TODO: Probably time to use some arg parser for this already


lerouxb · 2024-10-21T12:59:16Z

packages/compass-e2e-tests/helpers/test-runner-context.ts

+
+if (typeof electronPath !== 'string') {
+  throw new Error(
+    'Running e2e tests in an unsupported runtime: `electron` is not a string'


you mean electronPath?

lerouxb

Really nice cleanup! Long overdue too.

…runner This also includes re-applying changes from #6381 from scratch

gribnoysup · 2024-10-23T08:40:50Z

packages/compass-e2e-tests/helpers/test-runner-context.ts

+const TEST_ATLAS_CLOUD_EXTERNAL_DEFAULT_CONNECTIONS: ConnectionInfo[] | null =
+  JSON.parse(process.env.TEST_ATLAS_CLOUD_DEFAULT_CONNECTIONS ?? 'null');


@syn-zhu FYI, I'm changing this to be a stringified json array with connections following ConnectionInfo type schema because we're refactoring these STRING_1 / NAME_1 variables to be always derived from a list of connection info objects

gribnoysup · 2024-10-23T08:44:06Z

packages/compass-e2e-tests/helpers/compass.ts

+    // To be able to use `setCookies` method, we need to first open any page on
+    // the same domain as the cookies we are going to set
+    // https://webdriver.io/docs/api/browser/setCookies/
+    await browser.navigateTo(`${TEST_ATLAS_CLOUD_EXTERNAL_URL!}/404`);


@syn-zhu FYI, dropping the protocol from the url as this is a _URL, not just a domain

gribnoysup · 2024-10-23T08:44:43Z

packages/compass-e2e-tests/helpers/compass.ts

    await browser.setCookies(
      cookies
        .filter((cookie) => {
-          cookie.name.includes('mmsa-') ||


Not sure how it worked before, without a return this would just filter out all the cookies from the array

chore(e2e): restructure e2e test helpers to better separate shared st…

e244454

…ate and setup / teardown functions

gribnoysup commented Oct 21, 2024

View reviewed changes

gribnoysup added 2 commits October 21, 2024 13:42

chore: fix depcheck and var name

1b26fab

chore: remove TEST_PACKAGED_APP env var usage

c8e9dbd

lerouxb reviewed Oct 21, 2024

View reviewed changes

lerouxb approved these changes Oct 21, 2024

View reviewed changes

gribnoysup added 9 commits October 21, 2024 18:33

chore: replset for test servers and better cleanup

fc14ef6

chore: eslint fixes

1a0cd75

chore(e2e): use DEFAULT_CONNECTIONS in connect form helpers

926e3c5

chore: ts fix

4d911d1

chore: update to non-esm version

0667cb5

chore: fix glob

95a2cd3

chore: remove unused imports

5bb22e3

chore: more glob adjustments

0d6af31

Merge remote-tracking branch 'origin/main' into restructure-e2e-test-…

c921216

…runner This also includes re-applying changes from #6381 from scratch

gribnoysup commented Oct 23, 2024

View reviewed changes

chore: uncomment retries

566d9b9

gribnoysup commented Oct 23, 2024

View reviewed changes

gribnoysup changed the title ~~chore(e2e): restructure e2e test helpers to better separate shared state and setup / teardown functions~~ chore(e2e): restructure e2e test helpers to better separate shared state and setup / teardown functions COMPASS-8361 Oct 23, 2024

chore: fix env var name

8d73190

gribnoysup merged commit ce0d66f into main Oct 23, 2024
21 of 23 checks passed

gribnoysup deleted the restructure-e2e-test-runner branch October 23, 2024 11:52

		const TEST_ATLAS_CLOUD_EXTERNAL_DEFAULT_CONNECTIONS: ConnectionInfo[] \| null =
		JSON.parse(process.env.TEST_ATLAS_CLOUD_DEFAULT_CONNECTIONS ?? 'null');

chore(e2e): restructure e2e test helpers to better separate shared state and setup / teardown functions COMPASS-8361 #6383

chore(e2e): restructure e2e test helpers to better separate shared state and setup / teardown functions COMPASS-8361 #6383

Uh oh!

Conversation

gribnoysup commented Oct 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

lerouxb Oct 21, 2024 • 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!

lerouxb left a comment

Choose a reason for hiding this comment

Uh oh!

gribnoysup Oct 23, 2024 • 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gribnoysup commented Oct 21, 2024 •

edited

Loading

lerouxb Oct 21, 2024 •

edited

Loading

gribnoysup Oct 23, 2024 •

edited

Loading