refactor: Convert `openmct-yamcs` to ESModule #410

shefalijoshi · 2024-01-12T17:47:03Z

Closes #409

Describe your changes:

We need to update openmct-yamcs to match the cjs import syntax for tests.
Also, the tests changed in 3.3.0 and are updated with the new locators.
Added lint step
Added linting rules for ESModule Import
Remove Deploysentinel
Added e2e watch mode to match openmct
Update network tests to inspect the request/response

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?
Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

Changes address original issue?
Tests included and/or updated with changes?
Command line build passes?
Has this been smoke tested?
Testing instructions included in associated issue OR is this a dependency/testcase change?

Reviewer Checklist

Changes appear to address issue?
Reviewer has tested changes by following the provided instructions?
Changes appear not to be breaking changes?
Appropriate automated tests included?
Code style and in-line documentation are appropriate?
Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

…db overrides (like algorithms)

unlikelyzero · 2024-01-12T17:49:28Z

tests/e2e/yamcs/search.e2e.spec.js

-        await expect(page.locator('[aria-label="Search Result"] >> nth=1')).toContainText("CCSDS_Packet_Sequence.GroupFlags");
-        await expect(page.locator('[aria-label="Search Result"] >> nth=2')).toContainText("CCSDS_Packet_Sequence.Count");
+
+        await expect(page.getByLabel('Object Search Result').nth(0)).toContainText("CCSDS_Packet_Sequence");


test fixes to match 3.3.0

scottbell

Tested this in my branch, and code is good, but still looks like some tests are still failing?

example still WIP

unlikelyzero · 2024-01-17T20:44:26Z

package.json

@@ -19,6 +20,7 @@
    "test:e2e:smoke": "npx playwright test --config=./tests/e2e/playwright-quickstart.config.js --project=chromium quickstartSmoke",
    "test:e2e:quickstart": "npx playwright test --config=./tests/e2e/playwright-quickstart.config.js --project=chromium tests/e2e/yamcs/",
    "test:e2e:quickstart:local": "npx playwright test --config=./tests/e2e/playwright-quickstart.config.js --project=local-chrome tests/e2e/yamcs/",
+    "test:e2e:watch": "npx playwright test --ui --config=./tests/e2e/playwright-quickstart.config.js",


driveby to write tests faster

unlikelyzero · 2024-01-17T20:44:35Z

package.json

@@ -36,11 +38,11 @@
  "devDependencies": {
    "@babel/core": "7.20.12",
    "@babel/eslint-parser": "7.19.1",
-    "@deploysentinel/playwright": "0.3.4",


unlikelyzero · 2024-01-17T20:44:44Z

package.json

    "@playwright/test": "1.39.0",
    "babel-loader": "9.1.0",
    "babel-plugin-istanbul": "6.1.1",
    "eslint": "8.38.0",
+    "eslint-plugin-import":"2.29.1",


make sure we didn't miss anything

…henry/openmct-yamcs into fix-openmct-yamcs-build-tests

addressed

ozyx

I found a rogue .only()

ozyx · 2024-01-23T23:37:04Z

tests/e2e/yamcs/network.e2e.spec.js


-    test('Validate network traffic to YAMCS', async ({ page }) => {
+    test.only('Validate network traffic to YAMCS', async ({ page }) => {


ozyx · 2024-01-23T23:37:10Z

tests/e2e/yamcs/network.e2e.spec.js

        await page.goto("./", { waitUntil: "networkidle" });

+        await Promise.all([allParams, userGet, mdbOverride]);
+
+        //I'm not sure what this block does yet


@unlikelyzero I think the reason you're getting test failures here is that the page is doing a Promise.all on the page.waitForResponse after the await page.goto("./").

scottbell · 2024-01-24T10:39:14Z

check-optional-dependencies.js

-const fs = require('fs');
-const semver = require('semver');
-const myPackageJson = require('./package.json');
+import fs from 'node:fs';


we should be using fs/promises

scottbell · 2024-01-24T10:41:57Z

tests/e2e/yamcs/network.e2e.spec.js


+        await page.waitForLoadState('networkidle');
+        await new Promise(resolve => setTimeout(resolve, 500));


do we need this with the waitForResponse stuff?

scottbell · 2024-01-24T10:43:11Z

tests/e2e/yamcs/network.e2e.spec.js

+        await Promise.all([allParams, userGet, mdbOverride, parameterArchiveGet, batchPost, mdbOverride]);
+
+        // wait for debounced requests in YAMCS Latest Telemetry Provider to finish
+        await new Promise(resolve => setTimeout(resolve, 500));


same as above - is this still needed?

scottbell · 2024-01-24T10:47:19Z

tests/e2e/yamcs/network.e2e.spec.js

        await page.goto("./", { waitUntil: "networkidle" });

+        await Promise.all([allParams, userGet, mdbOverride]);
+
+        //I'm not sure what this block does yet


@unlikelyzero I think the reason you're getting test failures here is that the page is doing a Promise.all on the page.waitForResponse after the await page.goto("./").

addressed

shefalijoshi added 15 commits October 5, 2023 15:59

Check that the mdb override is of parameter type. Discard any other m…

65e5133

…db overrides (like algorithms)

Small change to logic

0db90a4

Merge branch 'master' into fix-mdb-overrides-parameter-check

e498811

Fix eslintrc filename

1622422

fix typo

4b761fc

fix imports

0a20d89

ESM modules

d9aa6a1

Redo webpack configs for ESM. Rename eslintrc

6cc816c

Fix webpack configs

e65d087

Fix imports extensions

7b59b7e

Fix optional dependencies check

3102e2c

Fix playwright esm

7b798d3

Fix search test locator

af4d3a4

Address review comments

096ad4a

Fix .js extensions for imports

aa728f7

shefalijoshi requested review from ozyx and unlikelyzero January 12, 2024 17:47

unlikelyzero reviewed Jan 12, 2024

View reviewed changes

Change file extension to .js

76d43a7

unlikelyzero requested a review from scottbell January 12, 2024 17:50

shefalijoshi added the pr:e2e:quickstart label Jan 12, 2024

unlikelyzero added this to the Target:3.3.0 milestone Jan 12, 2024

two locator fixes

c49d538

scottbell reviewed Jan 12, 2024

View reviewed changes

unlikelyzero added pr:e2e:quickstart and removed pr:e2e:quickstart labels Jan 12, 2024

unlikelyzero added 2 commits January 12, 2024 11:06

drive-by: remove deploysentinel

7745995

limit button

a87e4f4

unlikelyzero added pr:e2e:quickstart and removed pr:e2e:quickstart labels Jan 12, 2024

unlikelyzero and others added 5 commits January 16, 2024 19:54

add lint

3bffab7

add some linting

0de40c3

fix to example

34a8ac1

first pass update to e2e tests

6350336

fix export (to some degree)

c6eb7ef

example still WIP

unlikelyzero reviewed Jan 17, 2024

View reviewed changes

ozyx and others added 4 commits January 18, 2024 09:05

update

a0cc874

fix: webpack config

9bfa459

update tests

662f1c0

Merge branch 'fix-openmct-yamcs-build-tests' of https://github.com/ak…

749d381

…henry/openmct-yamcs into fix-openmct-yamcs-build-tests

unlikelyzero requested a review from ozyx January 23, 2024 23:34

ozyx previously requested changes Jan 23, 2024

View reviewed changes

unlikelyzero added the pr:e2e:quickstart label Jan 24, 2024

scottbell previously requested changes Jan 24, 2024

View reviewed changes

unlikelyzero added 2 commits January 24, 2024 12:18

update pathing

898bdc1

review comments

793d4ef

unlikelyzero requested a review from scottbell January 24, 2024 20:36

unlikelyzero requested a review from ozyx January 24, 2024 20:36

unlikelyzero added pr:e2e:quickstart and removed pr:e2e:quickstart labels Jan 24, 2024

github-actions bot removed the pr:e2e:quickstart label Jan 24, 2024

unlikelyzero modified the milestones: Target:3.3.0, Target:4.0.0 Jan 24, 2024

unlikelyzero merged commit 19513c3 into master Jan 24, 2024

unlikelyzero deleted the fix-openmct-yamcs-build-tests branch January 24, 2024 21:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: Convert `openmct-yamcs` to ESModule #410

refactor: Convert `openmct-yamcs` to ESModule #410

Uh oh!

shefalijoshi commented Jan 12, 2024 •

edited by unlikelyzero

Loading

Uh oh!

unlikelyzero Jan 12, 2024

Uh oh!

scottbell left a comment

Uh oh!

unlikelyzero Jan 17, 2024

Uh oh!

unlikelyzero Jan 17, 2024

Uh oh!

unlikelyzero Jan 17, 2024

Uh oh!

ozyx left a comment

Uh oh!

ozyx Jan 23, 2024

Uh oh!

ozyx Jan 23, 2024

Uh oh!

scottbell Jan 24, 2024

Uh oh!

scottbell Jan 24, 2024

Uh oh!

scottbell Jan 24, 2024

Uh oh!

scottbell Jan 24, 2024

Uh oh!

scottbell Jan 24, 2024

Uh oh!

Uh oh!


		test('Validate network traffic to YAMCS', async ({ page }) => {
		test.only('Validate network traffic to YAMCS', async ({ page }) => {


		await page.waitForLoadState('networkidle');
		await new Promise(resolve => setTimeout(resolve, 500));

refactor: Convert openmct-yamcs to ESModule #410

refactor: Convert openmct-yamcs to ESModule #410

Uh oh!

Conversation

shefalijoshi commented Jan 12, 2024 • edited by unlikelyzero Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottbell 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!

ozyx 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!

Uh oh!

refactor: Convert `openmct-yamcs` to ESModule #410

refactor: Convert `openmct-yamcs` to ESModule #410

shefalijoshi commented Jan 12, 2024 •

edited by unlikelyzero

Loading