Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and re-enable disabled unit test suites #6990

Merged
merged 35 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
6f04faf
fix: register event listeners etc in `created()` hook
ozyx Aug 15, 2023
24a98b7
fix: initialize `stalenessSubscription` before composition load and i…
ozyx Aug 15, 2023
435a09c
refactor(test): make `openmct` const
ozyx Aug 15, 2023
ceb206b
refactor: update overlayPlot spec to Vue 3 and fix tests
ozyx Aug 15, 2023
7247788
refactor: fix eslint errors
ozyx Aug 15, 2023
b7f800c
refactor: move initialization steps to `created()` hook
ozyx Aug 15, 2023
6909060
test: re-enable and fix stackedPlot test suite
ozyx Aug 15, 2023
9268eaa
fix: `hideTree=true` hides the tree again
ozyx Aug 18, 2023
0206f5d
fix: add back in check on mount
ozyx Aug 18, 2023
3698e4c
test: fix Layout tests
ozyx Aug 18, 2023
f2c768c
fix: BarGraph test
ozyx Aug 22, 2023
5fbe121
fix: plot inspector tests
ozyx Aug 22, 2023
f8f79cd
fix: reenable grand search tests
ozyx Aug 24, 2023
dfbc10d
fix: inspectorStyles test suite
ozyx Aug 24, 2023
ad95cc7
fix: re-enable most timeline tests
ozyx Aug 24, 2023
7d010de
fix: no need to hideTree in appactions
ozyx Aug 24, 2023
237df5c
fix: re-enable more tests
ozyx Aug 29, 2023
03c9f93
test: re-enable more tests
ozyx Aug 29, 2023
f8aa0b2
test: re-enable most plot tests
ozyx Aug 29, 2023
ef4b56c
chore: `lint:fix`
ozyx Aug 29, 2023
baa98e7
test: re-enable timelist suite
ozyx Aug 29, 2023
89d0413
fix(#7016): timers count down or up to a target date
ozyx Aug 30, 2023
b28e67c
test: add regression tests to cover disabled unit tests
ozyx Aug 30, 2023
93c0c63
refactor: lint:fix
ozyx Aug 30, 2023
8a3cb03
Merge branch 'master' into fix-unit-tests
scottbell Aug 31, 2023
309ca96
refactor: no need for momentjs here
ozyx Sep 1, 2023
104e489
fix: timerAction missed refactor
ozyx Sep 1, 2023
722f6fd
fix: ensure timestamp is always UTC string
ozyx Sep 1, 2023
d733910
test: use role selectors
ozyx Sep 1, 2023
200e8d3
docs: add instructions for clock override in e2e
ozyx Sep 1, 2023
6b26b58
docs: update
ozyx Sep 1, 2023
fc3504f
Update readme
unlikelyzero Sep 4, 2023
3d4ed3a
resolve conflicts
scottbell Sep 5, 2023
4fc4879
lint
scottbell Sep 5, 2023
d7a2969
spelling fixes
scottbell Sep 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,8 @@
"mediump",
"sinonjs",
"generatedata",
"grandsearch"
"grandsearch",
"websockets"
],
"dictionaries": ["npm", "softwareTerms", "node", "html", "css", "bash", "en_US"],
"ignorePaths": [
Expand Down
114 changes: 81 additions & 33 deletions e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,20 @@ To make this possible, we're leveraging a 3rd party service, [Percy](https://per

At present, we are using percy with two configuration files: `./e2e/.percy.nightly.yml` and `./e2e/.percy.ci.yml`. This is mainly to reduce the number of snapshots.

### (Advanced) Snapshot Testing
### Advanced: Snapshot Testing (Not Recommended)

Snapshot testing is very similar to visual testing but allows us to be more precise in detecting change without relying on a 3rd party service. Unfortuantely, this precision requires advanced test setup and teardown and so we're using this pattern as a last resort.
While snapshot testing offers a precise way to detect changes in your application without relying on third-party services like Percy.io, we've found that it doesn't offer any advantages over visual testing in our use-cases. Therefore, snapshot testing is **not recommended** for further implementation.

To give an example, if a _single_ visual test assertion for an Overlay plot is run through multiple DOM rendering engines at various viewports to see how the Plot looks. If that same test were run as a snapshot test, it could only be executed against a single browser, on a single platform (ubuntu docker container).
#### CI vs Manual Checks
Snapshot tests can be reliably executed in Continuous Integration (CI) environments but lack the manual oversight provided by visual testing platforms like Percy.io. This means they may miss issues that a human reviewer could catch during manual checks.

#### Example
A single visual test assertion in Percy.io can be executed across 10 different browser and resolution combinations without additional setup, providing comprehensive testing with minimal configuration. In contrast, a snapshot test is restricted to a single OS and browser resolution, requiring more effort to achieve the same level of coverage.


#### Further Reading
For those interested in the mechanics of snapshot testing with Playwright, you can refer to the [Playwright Snapshots Documentation](https://playwright.dev/docs/test-snapshots). However, keep in mind that we do not recommend using this approach.

Read more about [Playwright Snapshots](https://playwright.dev/docs/test-snapshots)

#### Open MCT's implementation

Expand Down Expand Up @@ -308,27 +315,42 @@ Skipping based on browser version (Rarely used): <https://github.com/microsoft/p

## Test Design, Best Practices, and Tips & Tricks

### Test Design (TODO)
### Test Design

#### Test as the User

In general, strive to test only through the UI as a user would. As stated in the [Playwright Best Practices](https://playwright.dev/docs/best-practices#test-user-visible-behavior):

- How to make tests robust to function in other contexts (VISTA, VIPER, etc.)
- Leverage the use of `appActions.js` methods such as `createDomainObjectWithDefaults()`
- How to make tests faster and more resilient
- When possible, navigate directly by URL:
> "Automated tests should verify that the application code works for the end users, and avoid relying on implementation details such as things which users will not typically use, see, or even know about such as the name of a function, whether something is an array, or the CSS class of some element. The end user will see or interact with what is rendered on the page, so your test should typically only see/interact with the same rendered output."

```javascript
By adhering to this principle, we can create tests that are both robust and reflective of actual user experiences.

#### How to make tests robust to function in other contexts (VISTA, COUCHDB, YAMCS, VIPER, etc.)
1. Leverage the use of `appActions.js` methods such as `createDomainObjectWithDefaults()`. This ensures that your tests will create unique instances of objects for your test to interact with.
1. Do not assert on the order or structure of objects available unless you created them yourself. These tests may be used against a persistent datastore like couchdb with many objects in the tree.
1. Do not search for your created objects. Open MCT does not performance uniqueness checks so it's possible that your tests will break when run twice.
1. Avoid creating locator aliases. This likely means that you're compensating for a bad locator. Improve the application instead.
1. Leverage `await page.goto('./', { waitUntil: 'domcontentloaded' });` instead of `{ waitUntil: 'networkidle' }`. Tests run against deployments with websockets often have issues with the networkidle detection.

#### How to make tests faster and more resilient
1. Avoid app interaction when possible. The best way of doing this is to navigate directly by URL:

```js
// You can capture the CreatedObjectInfo returned from this appAction:
const clock = await createDomainObjectWithDefaults(page, { type: 'Clock' });

// ...and use its `url` property to navigate directly to it later in the test:
await page.goto(clock.url);
```

- Leverage `await page.goto('./', { waitUntil: 'domcontentloaded' });`
1. Leverage `await page.goto('./', { waitUntil: 'domcontentloaded' });`
- Initial navigation should _almost_ always use the `{ waitUntil: 'domcontentloaded' }` option.
- Avoid repeated setup to test to test a single assertion. Write longer tests with multiple soft assertions.
1. Avoid repeated setup to test a single assertion. Write longer tests with multiple soft assertions.
This ensures that your changes will be picked up with large refactors.

### How to write a great test

- Avoid using css locators to find elements to the page. Use modern web accessible locators like `getByRole`
- Use our [App Actions](./appActions.js) for performing common actions whenever applicable.
- Use `waitForPlotsToRender()` before asserting against anything that is dependent upon plot series data being loaded and drawn.
- If you create an object outside of using the `createDomainObjectWithDefaults` App Action, make sure to fill in the 'Notes' section of your object with `page.testNotes`:
Expand All @@ -341,26 +363,29 @@ Skipping based on browser version (Rarely used): <https://github.com/microsoft/p
await notesInput.fill(testNotes);
```

#### How to write a great visual test

- Generally speaking, you should avoid being "specific" in what you hope to find in the diff. Visual tests are best suited for finding unknown unknowns.
- These should only use functional expect statements to verify assumptions about the state
in a test and not for functional verification of correctness. Visual tests are not supposed
to "fail" on assertions. Instead, they should be used to detect changes between builds or branches.
- A great visual test controls for the variation inherent to working with time-based telemetry and clocks. We do our best to remove this variation by using `percyCSS` to ignore all possible time-based components. For more, please see our [percyCSS file](./.percy.ci.yml).
- Additionally, you should try the following:
- Use fixed-time mode of Open MCT
- Use the `createExampleTelemetryObject` appAction to source telemetry
- When using the `createDomainObjectWithDefaults` appAction, make sure to specify a `name` which is explicit to avoid the autogenerated name
- Very likely, your test will not need to compare changes on the tree. Keep it out of the comparison with the following
- `await page.goto('./#/browse/mine')` will go to the root of the main view with the tree collapsed.
- If you only want to compare changes on a specific component, use the /visual/component/ folder and limit the scope of the comparison to the object like so:
- ```
await percySnapshot(page, `Tree Pane w/ single level expanded (theme: ${theme})`, {
scope: treePane
});
#### How to Write a Great Visual Test

1. **Look for the Unknown Unknowns**: Avoid asserting on specific differences in the visual diff. Visual tests are most effective for identifying unknown unknowns.

2. **Get the App into Interesting States**: Prioritize getting Open MCT into unusual layouts or behaviors before capturing a visual snapshot. For instance, you could open a dropdown menu.

3. **Expect the Unexpected**: Use functional expect statements only to verify assumptions about the state between steps. A great visual test doesn't fail during the test itself, but rather when changes are reviewed in Percy.io.

4. **Control Variability**: Account for variations inherent in working with time-based telemetry and clocks.
- Utilize `percyCSS` to ignore time-based elements. For more details, consult our [percyCSS file](./.percy.ci.yml).
- Use Open MCT's fixed-time mode.
- Employ the `createExampleTelemetryObject` appAction to source telemetry and specify a `name` to avoid autogenerated names.

5. **Hide the Tree and Inspector**: Generally, your test will not require comparisons involving the tree and inspector. These aspects are covered in component-specific tests (explained below). To exclude them from the comparison by default, navigate to the root of the main view with the tree and inspector hidden:
- `await page.goto('./#/browse/mine?hideTree=true&hideInspector=true')`

6. **Component-Specific Tests**: If you wish to focus on a particular component, use the `/visual/component/` folder and limit the scope of the comparison to that component. For instance:
```js
- The `scope` variable can be any valid css selector
await percySnapshot(page, `Tree Pane w/ single level expanded (theme: ${theme})`, {
scope: treePane
});
```
- Note: The `scope` variable can be any valid CSS selector.

#### How to write a great network test

Expand All @@ -377,12 +402,35 @@ For now, our best practices exist as self-tested, living documentation in our [e

For best practices with regards to mocking network responses, see our [couchdb.e2e.spec.js](./tests/functional/couchdb.e2e.spec.js) file.

### Tips & Tricks (TODO)
### Tips & Tricks

The following contains a list of tips and tricks which don't exactly fit into a FAQ or Best Practices doc.

- (Advanced) Overriding the Browser's Clock
It is possible to override the browser's clock in order to control time-based elements. Since this can cause unwanted behavior (i.e. Tree not rendering), only use this sparingly. To do this, use the `overrideClock` fixture as such:

```js
const { test, expect } = require('../../pluginFixtures.js');

test.describe('foo test suite', () => {

// All subsequent tests in this suite will override the clock
test.use({
clockOptions: {
now: 1732413600000, // A timestamp given as milliseconds since the epoch
shouldAdvanceTime: true // Should the clock tick?
}
});

test('bar test', async ({ page }) => {
// ...
});
});
```
More info and options for `overrideClock` can be found in [baseFixtures.js](baseFixtures.js)

- Working with multiple pages
There are instances where multiple browser pages will need to be opened to verify multi-page or multi-tab application behavior.
There are instances where multiple browser pages will needed to verify multi-page or multi-tab application behavior. Make sure to use the `@2p` annotation as well as name each page appropriately: i.e. `page1` and `page2` or `tab1` and `tab2` depending on the intended use case. Generally pages should be used unless testing `sharedWorker` code, specifically.

### Reporting

Expand Down
6 changes: 3 additions & 3 deletions e2e/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ async function createDomainObjectWithDefaults(

// Navigate to the parent object. This is necessary to create the object
// in the correct location, such as a folder, layout, or plot.
await page.goto(`${parentUrl}?hideTree=true`);
await page.goto(`${parentUrl}`);
unlikelyzero marked this conversation as resolved.
Show resolved Hide resolved

//Click the Create button
await page.click('button:has-text("Create")');
Expand Down Expand Up @@ -179,7 +179,7 @@ async function createPlanFromJSON(page, { name, json, parent = 'mine' }) {

// Navigate to the parent object. This is necessary to create the object
// in the correct location, such as a folder, layout, or plot.
await page.goto(`${parentUrl}?hideTree=true`);
await page.goto(`${parentUrl}`);

// Click the Create button
await page.click('button:has-text("Create")');
Expand Down Expand Up @@ -232,7 +232,7 @@ async function createExampleTelemetryObject(page, parent = 'mine') {
const name = 'VIPER Rover Heading';
const nameInputLocator = page.getByRole('dialog').locator('input[type="text"]');

await page.goto(`${parentUrl}?hideTree=true`);
await page.goto(`${parentUrl}`);

await page.locator('button:has-text("Create")').click();

Expand Down
92 changes: 92 additions & 0 deletions e2e/tests/functional/plugins/timer/timer.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,15 @@ const {
openObjectTreeContextMenu,
createDomainObjectWithDefaults
} = require('../../../../appActions');
import { MISSION_TIME } from '../../../../constants';

test.describe('Timer', () => {
let timer;

test.beforeEach(async ({ page }) => {
await page.goto('./', { waitUntil: 'domcontentloaded' });
timer = await createDomainObjectWithDefaults(page, { type: 'timer' });
await assertTimerElements(page, timer);
});

test('Can perform actions on the Timer', async ({ page }) => {
Expand Down Expand Up @@ -63,6 +66,70 @@ test.describe('Timer', () => {
});
});

test.describe('Timer with target date', () => {
let timer;

test.beforeEach(async ({ page }) => {
await page.goto('./', { waitUntil: 'domcontentloaded' });
timer = await createDomainObjectWithDefaults(page, { type: 'timer' });
await assertTimerElements(page, timer);
});

// Override clock
test.use({
ozyx marked this conversation as resolved.
Show resolved Hide resolved
clockOptions: {
now: MISSION_TIME,
shouldAdvanceTime: true
}
});

test('Can count down to a target date', async ({ page }) => {
// Set the target date to 2024-11-24 03:30:00
await page.getByTitle('More options').click();
await page.getByRole('menuitem', { name: /Edit Properties.../ }).click();
await page.getByPlaceholder('YYYY-MM-DD').fill('2024-11-24');
await page.locator('input[name="hour"]').fill('3');
await page.locator('input[name="min"]').fill('30');
await page.locator('input[name="sec"]').fill('00');
await page.getByLabel('Save').click();

// Get the current timer seconds value
const timerSecValue = (await page.locator('.c-timer__value').innerText()).split(':').at(-1);
await expect(page.locator('.c-timer__direction')).toHaveClass(/icon-minus/);

// Wait for the timer to count down and assert
await expect
.poll(async () => {
const newTimerValue = (await page.locator('.c-timer__value').innerText()).split(':').at(-1);
return Number(newTimerValue);
})
.toBeLessThan(Number(timerSecValue));
});

test('Can count up from a target date', async ({ page }) => {
// Set the target date to 2020-11-23 03:30:00
await page.getByTitle('More options').click();
await page.getByRole('menuitem', { name: /Edit Properties.../ }).click();
await page.getByPlaceholder('YYYY-MM-DD').fill('2020-11-23');
await page.locator('input[name="hour"]').fill('3');
await page.locator('input[name="min"]').fill('30');
await page.locator('input[name="sec"]').fill('00');
await page.getByLabel('Save').click();

// Get the current timer seconds value
const timerSecValue = (await page.locator('.c-timer__value').innerText()).split(':').at(-1);
await expect(page.locator('.c-timer__direction')).toHaveClass(/icon-plus/);

// Wait for the timer to count up and assert
await expect
.poll(async () => {
const newTimerValue = (await page.locator('.c-timer__value').innerText()).split(':').at(-1);
return Number(newTimerValue);
})
.toBeGreaterThan(Number(timerSecValue));
});
});

/**
* Actions that can be performed on a timer from context menus.
* @typedef {'Start' | 'Stop' | 'Pause' | 'Restart at 0'} TimerAction
Expand Down Expand Up @@ -141,14 +208,17 @@ function buttonTitleFromAction(action) {
* @param {TimerAction} action
*/
async function assertTimerStateAfterAction(page, action) {
const timerValue = page.locator('.c-timer__value');
let timerStateClass;
switch (action) {
case 'Start':
case 'Restart at 0':
timerStateClass = 'is-started';
expect(await timerValue.innerText()).toBe('0D 00:00:00');
break;
case 'Stop':
timerStateClass = 'is-stopped';
expect(await timerValue.innerText()).toBe('--:--:--');
break;
case 'Pause':
timerStateClass = 'is-paused';
Expand All @@ -157,3 +227,25 @@ async function assertTimerStateAfterAction(page, action) {

await expect.soft(page.locator('.c-timer')).toHaveClass(new RegExp(timerStateClass));
}

/**
* Assert that all the major components of a timer are present in the DOM.
* @param {import('@playwright/test').Page} page
* @param {import('../../../../appActions').CreatedObjectInfo} timer
*/
async function assertTimerElements(page, timer) {
const timerElement = page.locator('.c-timer');
ozyx marked this conversation as resolved.
Show resolved Hide resolved
const resetButton = page.getByRole('button', { name: 'Reset' });
const pausePlayButton = page
.getByRole('button', { name: 'Pause' })
.or(page.getByRole('button', { name: 'Start' }));
const timerDirectionIcon = page.locator('.c-timer__direction');
const timerValue = page.locator('.c-timer__value');

expect(await page.locator('.l-browse-bar__object-name').innerText()).toBe(timer.name);
expect(timerElement).toBeAttached();
expect(resetButton).toBeAttached();
expect(pausePlayButton).toBeAttached();
expect(timerDirectionIcon).toBeAttached();
expect(timerValue).toBeAttached();
}
3 changes: 1 addition & 2 deletions src/api/forms/components/controls/Datetime.vue
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ export default {
formatDatetime(timestamp = this.model.value) {
if (!timestamp) {
this.resetValues();

return;
}

Expand All @@ -137,7 +136,7 @@ export default {

const data = {
model,
value: timestamp
value: new Date(timestamp).toISOString()
};

this.$emit('onChange', data);
Expand Down
6 changes: 4 additions & 2 deletions src/plugins/charts/bar/BarGraphPlot.vue
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,15 @@ export default {
handler: 'updateData'
}
},
created() {
this.registerListeners();
},
mounted() {
this.plotResizeObserver.observe(this.$refs.plotWrapper);
Plotly.newPlot(this.$refs.plot, Array.from(this.data), this.getLayout(), {
responsive: true,
displayModeBar: false
});
this.registerListeners();
},
beforeUnmount() {
if (this.plotResizeObserver) {
Expand Down Expand Up @@ -226,7 +229,6 @@ export default {
window.dispatchEvent(new Event('resize'));
}, 250);
});
this.plotResizeObserver.observe(this.$refs.plotWrapper);
}
},
reset() {
Expand Down
Loading
Loading