From 32b68cf0df85cbad014050b6d51da57d455e87bb Mon Sep 17 00:00:00 2001 From: Scott Bell Date: Tue, 8 Aug 2023 18:49:39 +0200 Subject: [PATCH] cherry-pick(#6866): Only load annotations in fixed time mode or frozen (#6902) Only load annotations in fixed time mode or frozen (#6866) * fix annotations load to not happen on start * remove range checking per annotated point * back to local variable * reduce tagging size to improve reliability of test * remove .only * remove .only * reduce hz rate for functional test and keep high frequency test in performance * remove console.debugs * this test runs pretty fast now * fix network request tests to match new behavior --- e2e/appActions.js | 13 +- .../displayLayout/displayLayout.e2e.spec.js | 13 +- .../plugins/plot/tagging.e2e.spec.js | 44 +-- e2e/tests/performance/tagging.perf.spec.js | 273 ++++++++++++++++++ src/plugins/plot/MctPlot.vue | 12 +- src/plugins/plot/chart/MctChart.vue | 56 +--- 6 files changed, 350 insertions(+), 61 deletions(-) create mode 100644 e2e/tests/performance/tagging.perf.spec.js diff --git a/e2e/appActions.js b/e2e/appActions.js index 230602334b..9f64b089fc 100644 --- a/e2e/appActions.js +++ b/e2e/appActions.js @@ -35,6 +35,7 @@ * @property {string} type the type of domain object to create (e.g.: "Sine Wave Generator"). * @property {string} [name] the desired name of the created domain object. * @property {string | import('../src/api/objects/ObjectAPI').Identifier} [parent] the Identifier or uuid of the parent object. + * @property {Object} [customParameters] any additional parameters to be passed to the domain object's form. E.g. '[aria-label="Data Rate (hz)"]': {'0.1'} */ /** @@ -65,7 +66,10 @@ const { expect } = require('@playwright/test'); * @param {CreateObjectOptions} options * @returns {Promise} An object containing information about the newly created domain object. */ -async function createDomainObjectWithDefaults(page, { type, name, parent = 'mine' }) { +async function createDomainObjectWithDefaults( + page, + { type, name, parent = 'mine', customParameters = {} } +) { if (!name) { name = `${type}:${genUuid()}`; } @@ -94,6 +98,13 @@ async function createDomainObjectWithDefaults(page, { type, name, parent = 'mine await notesInput.fill(page.testNotes); } + // If there are any further parameters, fill them in + for (const [key, value] of Object.entries(customParameters)) { + const input = page.locator(`form[name="mctForm"] ${key}`); + await input.fill(''); + await input.fill(value); + } + // Click OK button and wait for Navigate event await Promise.all([ page.waitForLoadState(), diff --git a/e2e/tests/functional/plugins/displayLayout/displayLayout.e2e.spec.js b/e2e/tests/functional/plugins/displayLayout/displayLayout.e2e.spec.js index e231074076..ee093b45dc 100644 --- a/e2e/tests/functional/plugins/displayLayout/displayLayout.e2e.spec.js +++ b/e2e/tests/functional/plugins/displayLayout/displayLayout.e2e.spec.js @@ -260,6 +260,7 @@ test.describe('Display Layout', () => { test('When multiple plots are contained in a layout, we only ask for annotations once @couchdb', async ({ page }) => { + await setFixedTimeMode(page); // Create another Sine Wave Generator const anotherSineWaveObject = await createDomainObjectWithDefaults(page, { type: 'Sine Wave Generator' @@ -316,10 +317,20 @@ test.describe('Display Layout', () => { // wait for annotations requests to be batched and requested await page.waitForLoadState('networkidle'); - // Network requests for the composite telemetry with multiple items should be: // 1. a single batched request for annotations expect(networkRequests.length).toBe(1); + + await setRealTimeMode(page); + networkRequests = []; + + await page.reload(); + + // wait for annotations to not load (if we have any, we've got a problem) + await page.waitForLoadState('networkidle'); + + // In real time mode, we don't fetch annotations at all + expect(networkRequests.length).toBe(0); }); }); diff --git a/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js b/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js index aa3b6e5279..04d6ba2c2e 100644 --- a/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js +++ b/e2e/tests/functional/plugins/plot/tagging.e2e.spec.js @@ -32,7 +32,7 @@ const { waitForPlotsToRender } = require('../../../../appActions'); -test.describe.fixme('Plot Tagging', () => { +test.describe('Plot Tagging', () => { /** * Given a canvas and a set of points, tags the points on the canvas. * @param {import('@playwright/test').Page} page @@ -41,7 +41,7 @@ test.describe.fixme('Plot Tagging', () => { * @param {Number} yEnd a telemetry item with a plot * @returns {Promise} */ - async function createTags({ page, canvas, xEnd, yEnd }) { + async function createTags({ page, canvas, xEnd = 700, yEnd = 480 }) { await canvas.hover({ trial: true }); //Alt+Shift Drag Start to select some points to tag @@ -97,8 +97,8 @@ test.describe.fixme('Plot Tagging', () => { // click on the tagged plot point await canvas.click({ position: { - x: 325, - y: 377 + x: 100, + y: 100 } }); @@ -171,8 +171,6 @@ test.describe.fixme('Plot Tagging', () => { type: 'issue', description: 'https://github.com/nasa/openmct/issues/6822' }); - //Test.slow decorator is currently broken. Needs to be fixed in https://github.com/nasa/openmct/issues/5374 - test.slow(); const overlayPlot = await createDomainObjectWithDefaults(page, { type: 'Overlay Plot' @@ -181,13 +179,19 @@ test.describe.fixme('Plot Tagging', () => { const alphaSineWave = await createDomainObjectWithDefaults(page, { type: 'Sine Wave Generator', name: 'Alpha Sine Wave', - parent: overlayPlot.uuid + parent: overlayPlot.uuid, + customParameters: { + '[aria-label="Data Rate (hz)"]': '0.01' + } }); await createDomainObjectWithDefaults(page, { type: 'Sine Wave Generator', name: 'Beta Sine Wave', - parent: overlayPlot.uuid + parent: overlayPlot.uuid, + customParameters: { + '[aria-label="Data Rate (hz)"]': '0.02' + } }); await page.goto(overlayPlot.url); @@ -200,9 +204,7 @@ test.describe.fixme('Plot Tagging', () => { await createTags({ page, - canvas, - xEnd: 700, - yEnd: 480 + canvas }); await setFixedTimeMode(page); @@ -232,15 +234,15 @@ test.describe.fixme('Plot Tagging', () => { test('Tags work with Plot View of telemetry items', async ({ page }) => { await createDomainObjectWithDefaults(page, { - type: 'Sine Wave Generator' + type: 'Sine Wave Generator', + customParameters: { + '[aria-label="Data Rate (hz)"]': '0.01' + } }); - const canvas = page.locator('canvas').nth(1); await createTags({ page, - canvas, - xEnd: 700, - yEnd: 480 + canvas }); await basicTagsTests(page); }); @@ -253,13 +255,19 @@ test.describe.fixme('Plot Tagging', () => { const alphaSineWave = await createDomainObjectWithDefaults(page, { type: 'Sine Wave Generator', name: 'Alpha Sine Wave', - parent: stackedPlot.uuid + parent: stackedPlot.uuid, + customParameters: { + '[aria-label="Data Rate (hz)"]': '0.01' + } }); await createDomainObjectWithDefaults(page, { type: 'Sine Wave Generator', name: 'Beta Sine Wave', - parent: stackedPlot.uuid + parent: stackedPlot.uuid, + customParameters: { + '[aria-label="Data Rate (hz)"]': '0.02' + } }); await page.goto(stackedPlot.url); diff --git a/e2e/tests/performance/tagging.perf.spec.js b/e2e/tests/performance/tagging.perf.spec.js new file mode 100644 index 0000000000..2e3b7c32ba --- /dev/null +++ b/e2e/tests/performance/tagging.perf.spec.js @@ -0,0 +1,273 @@ +/***************************************************************************** + * Open MCT, Copyright (c) 2014-2023, United States Government + * as represented by the Administrator of the National Aeronautics and Space + * Administration. All rights reserved. + * + * Open MCT is licensed under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0. + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + * Open MCT includes source code licensed under additional open source + * licenses. See the Open Source Licenses file (LICENSES.md) included with + * this source code distribution or the Licensing information page available + * at runtime from the About dialog for additional information. + *****************************************************************************/ + +/* +Tests to verify plot tagging performance. +*/ + +const { test, expect } = require('../../pluginFixtures'); +const { + createDomainObjectWithDefaults, + setRealTimeMode, + setFixedTimeMode, + waitForPlotsToRender +} = require('../../appActions'); + +test.describe.fixme('Plot Tagging Performance', () => { + /** + * Given a canvas and a set of points, tags the points on the canvas. + * @param {import('@playwright/test').Page} page + * @param {HTMLCanvasElement} canvas a telemetry item with a plot + * @param {Number} xEnd a telemetry item with a plot + * @param {Number} yEnd a telemetry item with a plot + * @returns {Promise} + */ + async function createTags({ page, canvas, xEnd = 700, yEnd = 480 }) { + await canvas.hover({ trial: true }); + + //Alt+Shift Drag Start to select some points to tag + await page.keyboard.down('Alt'); + await page.keyboard.down('Shift'); + + await canvas.dragTo(canvas, { + sourcePosition: { + x: 1, + y: 1 + }, + targetPosition: { + x: xEnd, + y: yEnd + } + }); + + //Alt Drag End + await page.keyboard.up('Alt'); + await page.keyboard.up('Shift'); + + //Wait for canvas to stablize. + await canvas.hover({ trial: true }); + + // add some tags + await page.getByText('Annotations').click(); + await page.getByRole('button', { name: /Add Tag/ }).click(); + await page.getByPlaceholder('Type to select tag').click(); + await page.getByText('Driving').click(); + + await page.getByRole('button', { name: /Add Tag/ }).click(); + await page.getByPlaceholder('Type to select tag').click(); + await page.getByText('Science').click(); + } + + /** + * Given a telemetry item (e.g., a Sine Wave Generator) with a plot, tests that the plot can be tagged. + * @param {import('@playwright/test').Page} page + * @param {import('../../../../appActions').CreatedObjectInfo} telemetryItem a telemetry item with a plot + * @returns {Promise} + */ + async function testTelemetryItem(page, telemetryItem) { + // Check that telemetry item also received the tag + await page.goto(telemetryItem.url); + + await expect(page.getByText('No tags to display for this item')).toBeVisible(); + + const canvas = page.locator('canvas').nth(1); + + //Wait for canvas to stablize. + await canvas.hover({ trial: true }); + + // click on the tagged plot point + await canvas.click({ + position: { + x: 100, + y: 100 + } + }); + + await expect(page.getByText('Science')).toBeVisible(); + await expect(page.getByText('Driving')).toBeHidden(); + } + + /** + * Given a page, tests that tags are searchable, deletable, and persist across reloads. + * @param {import('@playwright/test').Page} page + * @returns {Promise} + */ + async function basicTagsTests(page) { + // Search for Driving + await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').click(); + + // Clicking elsewhere should cause annotation selection to be cleared + await expect(page.getByText('No tags to display for this item')).toBeVisible(); + + await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').fill('driv'); + // click on the search result + await page + .getByRole('searchbox', { name: 'OpenMCT Search' }) + .getByText(/Sine Wave/) + .first() + .click(); + + // Delete Driving + await page.hover('[aria-label="Tag"]:has-text("Driving")'); + await page.locator('[aria-label="Remove tag Driving"]').click(); + + // Search for Science + await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').click(); + await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').fill('sc'); + await expect(page.locator('[aria-label="Search Result"]').nth(0)).toContainText('Science'); + await expect(page.locator('[aria-label="Search Result"]').nth(0)).not.toContainText('Drilling'); + + // Search for Driving + await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').click(); + await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').fill('driv'); + await expect(page.getByText('No results found')).toBeVisible(); + + //Reload Page + await page.reload({ waitUntil: 'domcontentloaded' }); + // wait for plots to load + await waitForPlotsToRender(page); + + await page.getByText('Annotations').click(); + await expect(page.getByText('No tags to display for this item')).toBeVisible(); + + const canvas = page.locator('canvas').nth(1); + // click on the tagged plot point + await canvas.click({ + position: { + x: 100, + y: 100 + } + }); + + await expect(page.getByText('Science')).toBeVisible(); + await expect(page.getByText('Driving')).toBeHidden(); + } + + test.beforeEach(async ({ page }) => { + await page.goto('./', { waitUntil: 'domcontentloaded' }); + }); + + test('Tags work with Overlay Plots', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/nasa/openmct/issues/6822' + }); + //Test.slow decorator is currently broken. Needs to be fixed in https://github.com/nasa/openmct/issues/5374 + test.slow(); + + const overlayPlot = await createDomainObjectWithDefaults(page, { + type: 'Overlay Plot' + }); + + const alphaSineWave = await createDomainObjectWithDefaults(page, { + type: 'Sine Wave Generator', + name: 'Alpha Sine Wave', + parent: overlayPlot.uuid + }); + + await createDomainObjectWithDefaults(page, { + type: 'Sine Wave Generator', + name: 'Beta Sine Wave', + parent: overlayPlot.uuid + }); + + await page.goto(overlayPlot.url); + + let canvas = page.locator('canvas').nth(1); + + // Switch to real-time mode + // Adding tags should pause the plot + await setRealTimeMode(page); + + await createTags({ + page, + canvas + }); + + await setFixedTimeMode(page); + + await basicTagsTests(page); + await testTelemetryItem(page, alphaSineWave); + + // set to real time mode + await setRealTimeMode(page); + + // Search for Science + await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').click(); + await page.locator('[aria-label="OpenMCT Search"] input[type="search"]').fill('sc'); + // click on the search result + await page + .getByRole('searchbox', { name: 'OpenMCT Search' }) + .getByText('Alpha Sine Wave') + .first() + .click(); + // wait for plots to load + await expect(page.locator('.js-series-data-loaded')).toBeVisible(); + // expect plot to be paused + await expect(page.locator('[title="Resume displaying real-time data"]')).toBeVisible(); + + await setFixedTimeMode(page); + }); + + test('Tags work with Plot View of telemetry items', async ({ page }) => { + await createDomainObjectWithDefaults(page, { + type: 'Sine Wave Generator' + }); + const canvas = page.locator('canvas').nth(1); + await createTags({ + page, + canvas + }); + await basicTagsTests(page); + }); + + test('Tags work with Stacked Plots', async ({ page }) => { + const stackedPlot = await createDomainObjectWithDefaults(page, { + type: 'Stacked Plot' + }); + + const alphaSineWave = await createDomainObjectWithDefaults(page, { + type: 'Sine Wave Generator', + name: 'Alpha Sine Wave', + parent: stackedPlot.uuid + }); + + await createDomainObjectWithDefaults(page, { + type: 'Sine Wave Generator', + name: 'Beta Sine Wave', + parent: stackedPlot.uuid + }); + + await page.goto(stackedPlot.url); + + const canvas = page.locator('canvas').nth(1); + + await createTags({ + page, + canvas, + xEnd: 700, + yEnd: 215 + }); + await basicTagsTests(page); + await testTelemetryItem(page, alphaSineWave); + }); +}); diff --git a/src/plugins/plot/MctPlot.vue b/src/plugins/plot/MctPlot.vue index 3123041cd7..35f7fa362e 100644 --- a/src/plugins/plot/MctPlot.vue +++ b/src/plugins/plot/MctPlot.vue @@ -248,6 +248,7 @@ export default { highlights: [], annotatedPoints: [], annotationSelections: [], + annotationsEverLoaded: false, lockHighlightPoint: false, yKeyOptions: [], yAxisLabel: '', @@ -394,7 +395,11 @@ export default { ); this.openmct.objectViews.on('clearData', this.clearData); - this.$on('loadingComplete', this.loadAnnotations); + this.$on('loadingComplete', () => { + if (this.annotationViewingAndEditingAllowed) { + this.loadAnnotations(); + } + }); this.openmct.selection.on('change', this.updateSelection); this.yAxisListWithRange = [this.config.yAxis, ...this.config.additionalYAxes]; @@ -636,6 +641,7 @@ export default { if (rawAnnotationsForPlot) { this.annotatedPoints = this.findAnnotationPoints(rawAnnotationsForPlot); } + this.annotationsEverLoaded = true; }, loadSeriesData(series) { //this check ensures that duplicate requests don't happen on load @@ -789,6 +795,7 @@ export default { }; this.config.xAxis.set('range', newRange); if (!isTick) { + this.annotatedPoints = []; this.clearPanZoomHistory(); this.synchronizeIfBoundsMatch(); this.loadMoreData(newRange, true); @@ -1785,6 +1792,9 @@ export default { }); this.config.xAxis.set('frozen', true); this.setStatus(); + if (!this.annotationsEverLoaded) { + this.loadAnnotations(); + } }, resumeRealtimeData() { diff --git a/src/plugins/plot/chart/MctChart.vue b/src/plugins/plot/chart/MctChart.vue index 1cc3438aa2..3bb67246dd 100644 --- a/src/plugins/plot/chart/MctChart.vue +++ b/src/plugins/plot/chart/MctChart.vue @@ -826,56 +826,32 @@ export default { ); } }, - annotatedPointWithinRange(annotatedPoint, xRange, yRange) { - if (!yRange) { - return false; - } - - const xValue = annotatedPoint.series.getXVal(annotatedPoint.point); - const yValue = annotatedPoint.series.getYVal(annotatedPoint.point); - - return ( - xValue > xRange.min && xValue < xRange.max && yValue > yRange.min && yValue < yRange.max - ); - }, drawAnnotatedPoints(yAxisId) { // we should do this by series, and then plot all the points at once instead // of doing it one by one if (this.annotatedPoints && this.annotatedPoints.length) { const uniquePointsToDraw = []; - const xRange = this.config.xAxis.get('displayRange'); - let yRange; - if (yAxisId === this.config.yAxis.get('id')) { - yRange = this.config.yAxis.get('displayRange'); - } else if (this.config.additionalYAxes.length) { - const yAxisForId = this.config.additionalYAxes.find( - (yAxis) => yAxis.get('id') === yAxisId - ); - yRange = yAxisForId.get('displayRange'); - } const annotatedPoints = this.annotatedPoints.filter( this.matchByYAxisId.bind(this, yAxisId) ); annotatedPoints.forEach((annotatedPoint) => { - // if the annotation is outside the range, don't draw it - if (this.annotatedPointWithinRange(annotatedPoint, xRange, yRange)) { - const canvasXValue = this.offset[yAxisId].xVal( - annotatedPoint.point, - annotatedPoint.series - ); - const canvasYValue = this.offset[yAxisId].yVal( - annotatedPoint.point, - annotatedPoint.series - ); - const pointToDraw = new Float32Array([canvasXValue, canvasYValue]); - const drawnPoint = uniquePointsToDraw.some((rawPoint) => { - return rawPoint[0] === pointToDraw[0] && rawPoint[1] === pointToDraw[1]; - }); - if (!drawnPoint) { - uniquePointsToDraw.push(pointToDraw); - this.drawAnnotatedPoint(annotatedPoint, pointToDraw); - } + // annotation points are all within range (checked in MctPlot with FlatBush), so we don't need to check + const canvasXValue = this.offset[yAxisId].xVal( + annotatedPoint.point, + annotatedPoint.series + ); + const canvasYValue = this.offset[yAxisId].yVal( + annotatedPoint.point, + annotatedPoint.series + ); + const pointToDraw = new Float32Array([canvasXValue, canvasYValue]); + const drawnPoint = uniquePointsToDraw.some((rawPoint) => { + return rawPoint[0] === pointToDraw[0] && rawPoint[1] === pointToDraw[1]; + }); + if (!drawnPoint) { + uniquePointsToDraw.push(pointToDraw); + this.drawAnnotatedPoint(annotatedPoint, pointToDraw); } }); }