Skip to content

Commit

Permalink
Memory leak fixes for several views (#7057)
Browse files Browse the repository at this point in the history
* Change the mount utility to use Vue's createApp and defineComponent methods

* Fix display layout memory leaks caused by `getSelectionContext`

* fix some display layout leaks due to use of slots

* Fix imagery memory leak (removed span tag). NOTE: CompassRose svg leaks memory - must test on firefox to see if this is a Chrome leak.

* Fix ActionsAPI action collection and applicable actions leak.

* Fix flexible layout memory leaks - remove listeners on unmount. NOTE: One type of overlay plot (Rover Yaw) is still leaking.

* pass in the el on mount

* e2e test config and spec changes

* Remove mounting of limit lines. Use components directly

* test: remove `.only()`

* Fix display layout memory leaks

* Enable passing tests

* e2e README and appActions should be what master has.

* lint: add word to cspell list

* lint: fixes

* lint:fix

* fix: revert `el` change

* fix: remove empty span

* fix: creating shapes in displayLayout

* fix: avoid `splice` as it loses reactivity

* test: reduce timeout time

* quick fixes

* add prod mode and convert the test config to select the correct mode

* Fix webpack prod config

* Add launch flag for exposing window.gc

* never worked

* explicit naming

* rename

* We don't need to destroy view providers

* test: increase timeout time

* test: unskip all mem tests

* fix(vue-loader): disable static hoisting

* chore: run `test:perf:memory`

* Don't destroy view providers

* Move context menu once listener to beforeUnmount instead.

* Disconnect all resize observers on unmount

* Delete Test vue component

* Use beforeUnmount and remove splice(0) in favor of [] for emptying arrays

* re-structure

* fix: unregister listener in pane.vue

* test: tweak timeouts

* chore: lint:fix

* test: unskip perf tests

* fix: unregister events properly

* fix: unregister listener

* fix: unregister listener

* fix: unregister listener

* fix: use `unmounted()`

* fix: unregister listeners

* fix: unregister listener properly

* chore: lint:fix

* test: fix imagery layer toggle test

* test: increase timeout

* Don't use anonymous functions for listeners

* Destroy objects and event listeners properly

* Delete config stores that are created by components

* Use the right unmount hook. Destroy mounted view on unmount.

* Use unmounted, not beforeUnmounted

* Lint fixes

* Fix time strip memory leak

* Undo unneeded change for memory leaks.

* chore: combine common webpack configs

---------

Co-authored-by: Jesse Mazzella <[email protected]>
Co-authored-by: John Hill <[email protected]>
  • Loading branch information
3 people authored Sep 20, 2023
1 parent 61e7050 commit b8949db
Show file tree
Hide file tree
Showing 65 changed files with 887 additions and 393 deletions.
4 changes: 3 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,9 @@ jobs:
steps:
- build_and_install:
node-version: <<parameters.node-version>>
- run: npm run test:perf
- run: npm run test:perf:memory
- run: npm run test:perf:localhost
- run: npm run test:perf:contract
- store_test_results:
path: test-results/results.xml
- store_artifacts:
Expand Down
5 changes: 4 additions & 1 deletion .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,10 @@
"sinonjs",
"generatedata",
"grandsearch",
"websockets"
"websockets",
"swgs",
"memlab",
"devmode"
],
"dictionaries": ["npm", "softwareTerms", "node", "html", "css", "bash", "en_US"],
"ignorePaths": [
Expand Down
11 changes: 11 additions & 0 deletions .webpack/webpack.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ const projectRootDir = path.resolve(__dirname, '..');
/** @type {import('webpack').Configuration} */
const config = {
context: projectRootDir,
devServer: {
client: {
progress: true,
overlay: {
// Disable overlay for runtime errors.
// See: https://github.com/webpack/webpack-dev-server/issues/4771
runtimeErrors: false
}
}
},
entry: {
openmct: './openmct.js',
generatorWorker: './example/generator/generatorWorker.js',
Expand Down Expand Up @@ -125,6 +135,7 @@ const config = {
loader: 'vue-loader',
options: {
compilerOptions: {
hoistStatic: false,
whitespace: 'preserve',
compatConfig: {
MODE: 2
Expand Down
8 changes: 0 additions & 8 deletions .webpack/webpack.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,6 @@ module.exports = merge(common, {
directory: path.join(__dirname, '..', '/dist'),
publicPath: '/dist',
watch: false
},
client: {
progress: true,
overlay: {
// Disable overlay for runtime errors.
// See: https://github.com/webpack/webpack-dev-server/issues/4771
runtimeErrors: false
}
}
}
});
14 changes: 9 additions & 5 deletions e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,11 @@ npm run test:e2e:updatesnapshots

## Performance Testing

The open source performance tests function mostly as a contract for the locator logic, functionality, and assumptions will work in our downstream, closed source test suites.
The open source performance tests function in three ways which match their naming and folder structure:

They're found under `./e2e/tests/performance` and are to be executed with the following npm script:

`npm run test:perf`
`./e2e/tests/performance` - The tests at the root of this folder path detect functional changes which are mostly apparent with large performance regressions like [this](https://github.com/nasa/openmct/issues/6879). These tests run against openmct webpack in `production-mode` with the `npm run test:perf:localhost` script.
`./e2e/tests/performance/contract/` - These tests serve as [contracts](https://martinfowler.com/bliki/ContractTest.html) for the locator logic, functionality, and assumptions will work in our downstream, closed source test suites. These tests run against openmct webpack in `dev-mode` with the `npm run test:perf:contract` script.
`./e2e/tests/performance/memory/` - These tests execute memory leak detection checks in various ways. This is expected to evolve as we move to the `memlab` project. These tests run against openmct webpack in `production-mode` with the `npm run test:perf:memory` script.

These tests are expected to become blocking and gating with assertions as we extend the capabilities of Playwright.

Expand All @@ -158,8 +158,11 @@ Our file structure follows the type of type of testing being excercised at the e
|`./tests/functional/example/` | Tests which specifically verify the example plugins (e.g.: Sine Wave Generator).|
|`./tests/functional/plugins/` | Tests which loosely test each plugin. This folder is the most likely to change. Note: some `@snapshot` tests are still contained within this structure.|
|`./tests/framework/` | Tests which verify that our testing framework's functionality and assumptions will continue to work based on further refactoring or Playwright version changes (e.g.: verifying custom fixtures and appActions).|
|`./tests/performance/` | Performance tests.|
|`./tests/performance/` | Performance tests which should be run on every commit.|
|`./tests/performance/contract/` | A subset of performance tests which are designed to provide a contract between the open source tests which are run on every commit and the downstream tests which are run post merge and with other frameworks.|
|`./tests/performance/memory` | A subset of performance tests which are designed to test for memory leaks.|
|`./tests/visual/` | Visual tests.|
|`./tests/visual/component/` | Visual tests which are only run against a single component.|
|`./appActions.js` | Contains common methods which can be leveraged by test case authors to quickly move through the application when writing new tests.|
|`./baseFixture.js` | Contains base fixtures which only extend default `@playwright/test` functionality. The expectation is that these fixtures will be removed as the native Playwright API improves|

Expand All @@ -176,6 +179,7 @@ Open MCT is leveraging the [config file](https://playwright.dev/docs/test-config
|`./playwright-ci.config.js` | Used when running in CI or to debug CI issues locally|
|`./playwright-local.config.js` | Used when running locally|
|`./playwright-performance.config.js` | Used when running performance tests in CI or locally|
|`./playwright-performance-devmode.config.js` | Used when running performance tests in CI or locally|
|`./playwright-visual.config.js` | Used to run the visual tests in CI or locally|

#### Test Tags
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,32 @@
// playwright.config.js
// @ts-check

const CI = process.env.CI === 'true';

/** @type {import('@playwright/test').PlaywrightTestConfig} */
const config = {
retries: 1, //Only for debugging purposes for trace: 'on-first-retry'
testDir: 'tests/performance/',
testMatch: '*.contract.perf.spec.js', //Run everything except contract tests which require marks in dev mode
timeout: 60 * 1000,
workers: 1, //Only run in serial with 1 worker
webServer: {
command: 'npm run start', //coverage not generated
command: 'npm run start', //need development mode for performance.marks and others
url: 'http://localhost:8080/#',
timeout: 200 * 1000,
reuseExistingServer: !CI
reuseExistingServer: false
},
use: {
browserName: 'chromium',
baseURL: 'http://localhost:8080/',
headless: CI, //Only if running locally
ignoreHTTPSErrors: true,
headless: true,
ignoreHTTPSErrors: false, //HTTP performance varies!
screenshot: 'off',
trace: 'on-first-retry',
video: 'off'
},
projects: [
{
name: 'chrome',
testIgnore: '*.memory.perf.spec.js', //Do not run memory tests without proper flags. Shouldn't get here
use: {
browserName: 'chromium'
}
Expand Down
60 changes: 60 additions & 0 deletions e2e/playwright-performance-prod.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/* eslint-disable no-undef */
// playwright.config.js
// @ts-check

/** @type {import('@playwright/test').PlaywrightTestConfig} */
const config = {
retries: 0, //Only for debugging purposes for trace: 'on-first-retry'
testDir: 'tests/performance/',
testIgnore: '*.contract.perf.spec.js', //Run everything except contract tests which require marks in dev mode
timeout: 60 * 1000,
workers: 1, //Only run in serial with 1 worker
webServer: {
command: 'npm run start:prod', //Production mode
url: 'http://localhost:8080/#',
timeout: 200 * 1000,
reuseExistingServer: false //Must be run with this option to prevent dev mode
},
use: {
baseURL: 'http://localhost:8080/',
headless: true,
ignoreHTTPSErrors: false, //HTTP performance varies!
screenshot: 'off',
trace: 'on-first-retry',
video: 'off'
},
projects: [
{
name: 'chrome-memory',
testMatch: '*.memory.perf.spec.js', //Only run memory tests
use: {
browserName: 'chromium',
launchOptions: {
args: [
'--no-sandbox',
'--disable-notifications',
'--use-fake-ui-for-media-stream',
'--use-fake-device-for-media-stream',
'--js-flags=--no-move-object-start --expose-gc',
'--enable-precise-memory-info',
'--display=:100'
]
}
}
},
{
name: 'chrome',
testIgnore: '*.memory.perf.spec.js', //Do not run memory tests without proper flags
use: {
browserName: 'chromium'
}
}
],
reporter: [
['list'],
['junit', { outputFile: '../test-results/results.xml' }],
['json', { outputFile: '../test-results/results.json' }]
]
};

module.exports = config;
1 change: 1 addition & 0 deletions e2e/test-data/memory-leak-detection.json

Large diffs are not rendered by default.

File renamed without changes.
File renamed without changes.
121 changes: 0 additions & 121 deletions e2e/tests/performance/memleak-imagery.perf.spec.js

This file was deleted.

Loading

0 comments on commit b8949db

Please sign in to comment.