Gauge fixes for NaN and composition policy #5608

charlesh88 · 2022-08-04T18:14:14Z

Closes #5536
Closes #5538

Describe your changes:

Significant changes to code flow for better handling of missing telemetry values to handle NaN problem. Closes Gauge displays NaN when telemetry not available #5538.
Changes to flow to handle range display when updating composition and when ranges are set by limits.
Added GaugeCompositionPolicy.js; closes Gauge allows bad drag and drop #5536.

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?
Changes appear not to be breaking changes?
Appropriate unit tests included?
Code style and in-line documentation are appropriate?
Commit messages meet standards?
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)

- Significant changes to code flow for better handling of missing telemetry values; closes #5538. - Changes to flow to handle range display when updating composition and when ranges are set by limits. - Added `GaugeCompositionPolicy.js`; closes #5536.

- Code cleanup, linting.

codecov · 2022-08-04T18:19:28Z

Codecov Report

Merging #5608 (5a561be) into master (cdd772a) will decrease coverage by 14.13%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

❗ Current head 5a561be differs from pull request most recent head 2c48b0d. Consider uploading reports for the commit 2c48b0d to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #5608       +/-   ##
===========================================
- Coverage   55.96%   41.83%   -14.13%     
===========================================
  Files         653      415      -238     
  Lines       26213    12960    -13253     
  Branches     2525        0     -2525     
===========================================
- Hits        14669     5422     -9247     
+ Misses      10839     7538     -3301     
+ Partials      705        0      -705

Flag	Coverage Δ
e2e-full	`41.83% <ø> (-0.09%)`	⬇️
e2e-stable	`?`
unit	`?`

Files	Coverage Δ
src/plugins/gauge/GaugePlugin.js	`20.00% <ø> (-20.00%)`	⬇️

... and 523 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a914e4f...2c48b0d. Read the comment docs.

src/plugins/gauge/GaugeCompositionPolicy.js

src/plugins/gauge/components/Gauge.vue

scottbell

Good work @charlesh88! I had a few comments below

akhenry · 2022-10-17T20:56:53Z

@charlesh88 This has been reviewed, waiting for followup from you.

davetsay · 2023-01-30T22:00:31Z

@charlesh88 to review comments by @scottbell

- Merge master, resolve conflicts.

deploysentinel · 2023-10-24T22:36:30Z

Current Playwright Test Results Summary

✅ 161 Passing - ⚠️ 2 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 11/16/2023 06:40:32pm UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: 2c48b0d

Started: 11/16/2023 06:34:15pm UTC

⚠️ Flakes

📄 functional/plugins/notebook/restrictedNotebook.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Restricted Notebook with a page locked and with an embed @addinit Disallows embeds to be deleted if page locked @addinit Retry 1 • Initial Attempt	0% (0) 0 / 56 runs failed over last 7 days	57.14% (32) 32 / 56 runs flaked over last 7 days

📄 functional/plugins/notebook/notebookSnapshots.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Snapshot image tests Can drop an image onto a notebook and create a new entry Retry 1 • Initial Attempt	0% (0) 0 / 57 runs failed over last 7 days	73.68% (42) 42 / 57 runs flaked over last 7 days

View Detailed Build Results

Current Playwright Test Results Summary

✅ 14 Passing - ⚠️ 1 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 11/16/2023 06:40:32pm UTC)

Run Details

Running Workflow e2e-couchdb on Github Actions

Commit: 2c48b0d

Started: 11/16/2023 06:34:59pm UTC

⚠️ Flakes

📄 functional/couchdb.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
CouchDB Status Indicator with mocked responses @couchdb Shows red if not connected Retry 1 • Initial Attempt	0% (0) 0 / 32 runs failed over last 7 days	21.88% (7) 7 / 32 runs flaked over last 7 days

View Detailed Build Results

- Linting fixes.

charlesh88 · 2023-11-06T22:11:01Z

@ozyx Says run lint fix on this, and it should go right in.

…ge-5538

ozyx

@charlesh88 to add an e2e test

- Added test for 'Gauge does not display NaN when data not available'.

- Refined test.

- Refined 'NaN' test. - Added test for 'Gauge enforces composition policy';

- Fix linting issues.

charlesh88 · 2023-11-07T21:56:00Z

@ozyx e2e tests added, and files have been linted. Ready for a final look.

ozyx

Awesome work! I've got some suggestions to improve the e2e tests.

e2e/tests/functional/plugins/gauge/gauge.e2e.spec.js

src/plugins/gauge/GaugeCompositionPolicy.js

src/plugins/gauge/GaugePlugin.js

src/plugins/gauge/components/GaugeComponent.vue

- Suggested changes from PR review.

scottbell

Nice work @charlesh88, and thank you for adding the tests. Below are the functionality results.

For the composition issue, it looks resolved. Here's before:

Composition-before.mov

and after it's properly preventing non-telemetry from being dropped on it:

Composition-after.mov

It still does allow non-numerical telemetry points (e.g., events), but not sure that should hold up the PR.

The NaN issue seems already resolved? Here's before with a delayed Sine Wave Generator set to a 3s delay:

NaN-before.mov

And here's after which is showing the same behavior:

NaN-after.mov

…ge-5538

ozyx

Looks great! Just one small fix to get the e2e test passing

e2e/tests/functional/plugins/gauge/gauge.e2e.spec.js

Co-authored-by: Jesse Mazzella <ozyx@users.noreply.github.com>

charlesh88 added 3 commits August 3, 2022 18:23

Closes #5536, #5538

3986a12

- Significant changes to code flow for better handling of missing telemetry values; closes #5538. - Changes to flow to handle range display when updating composition and when ranges are set by limits. - Added `GaugeCompositionPolicy.js`; closes #5536.

Merge remote-tracking branch 'origin' into fix-gauge-5538

40597ff

Closes #5536, #5538

06058c4

- Code cleanup, linting.

charlesh88 requested review from akhenry and scottbell August 4, 2022 18:32

scottbell reviewed Aug 5, 2022

View reviewed changes

src/plugins/gauge/GaugeCompositionPolicy.js Outdated Show resolved Hide resolved

scottbell reviewed Aug 5, 2022

View reviewed changes

src/plugins/gauge/components/Gauge.vue Outdated Show resolved Hide resolved

scottbell reviewed Aug 5, 2022

View reviewed changes

akhenry removed their request for review August 15, 2022 21:09

Merge branch 'master' into fix-gauge-5538

0c6e385

davetsay added this to the Target:2.2.0 milestone Jan 30, 2023

unlikelyzero removed this from the Target:2.2.0 milestone Mar 14, 2023

Closes #5538

e8b4b8f

- Merge master, resolve conflicts.

charlesh88 enabled auto-merge (squash) October 24, 2023 23:34

davetsay and others added 4 commits October 26, 2023 10:11

Merge branch 'master' into fix-gauge-5538

3b6dc26

Closes #5538

f19c0c3

- Linting fixes.

Merge branch 'master' into fix-gauge-5538

69ca440

Merge branch 'master' into fix-gauge-5538

665ac24

Merge branch 'fix-gauge-5538' of github.com:nasa/openmct into fix-gau…

26f1916

…ge-5538

ozyx self-requested a review November 6, 2023 22:59

ozyx suggested changes Nov 6, 2023

View reviewed changes

charlesh88 added 4 commits November 6, 2023 15:56

Closes #5538

300100a

- Added test for 'Gauge does not display NaN when data not available'.

Closes #5538

8979ed3

- Refined test.

Closes #5538

cf40952

- Refined 'NaN' test. - Added test for 'Gauge enforces composition policy';

Closes #5538

fb49b0b

- Fix linting issues.

Merge branch 'master' into fix-gauge-5538

f0a0faf

charlesh88 requested a review from ozyx November 7, 2023 21:44

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Nov 7, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Nov 7, 2023

ozyx suggested changes Nov 7, 2023

View reviewed changes

charlesh88 added 2 commits November 7, 2023 15:42

Closes #5538

7682062

- Suggested changes from PR review.

Closes #5538

5ddbdaf

- Suggested changes from PR review.

scottbell approved these changes Nov 8, 2023

View reviewed changes

charlesh88 added 3 commits November 8, 2023 10:47

Merge branch 'master' into fix-gauge-5538

9b051ca

Merge remote-tracking branch 'origin' into fix-gauge-5538

f19a06d

Merge branch 'fix-gauge-5538' of github.com:nasa/openmct into fix-gau…

5bf67b8

…ge-5538

ozyx suggested changes Nov 9, 2023

View reviewed changes

e2e/tests/functional/plugins/gauge/gauge.e2e.spec.js Show resolved Hide resolved

e2e/tests/functional/plugins/gauge/gauge.e2e.spec.js Outdated Show resolved Hide resolved

ozyx disabled auto-merge November 16, 2023 18:11

charlesh88 and others added 3 commits November 16, 2023 10:12

Update e2e/tests/functional/plugins/gauge/gauge.e2e.spec.js

1f58050

Co-authored-by: Jesse Mazzella <ozyx@users.noreply.github.com>

chore: lint:fix

ac1a105

Merge branch 'master' into fix-gauge-5538

b596f32

ozyx added the pr:e2e:couchdb npm run test:e2e:couchdb label Nov 16, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Nov 16, 2023

Merge branch 'master' into fix-gauge-5538

2c48b0d

ozyx added the pr:e2e:couchdb npm run test:e2e:couchdb label Nov 16, 2023

ozyx enabled auto-merge (squash) November 16, 2023 18:32

ozyx approved these changes Nov 16, 2023

View reviewed changes

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Nov 16, 2023

ozyx merged commit 15ee830 into master Nov 16, 2023
12 of 13 checks passed

ozyx deleted the fix-gauge-5538 branch November 16, 2023 18:40

unlikelyzero added this to the Target:3.2.0 milestone Nov 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gauge fixes for NaN and composition policy #5608

Gauge fixes for NaN and composition policy #5608

charlesh88 commented Aug 4, 2022 •

edited by ozyx

Loading

codecov bot commented Aug 4, 2022 •

edited

Loading

scottbell left a comment

akhenry commented Oct 17, 2022

davetsay commented Jan 30, 2023

deploysentinel bot commented Oct 24, 2023 •

edited

Loading

Test Case Results

Test Case Results

Test Case Results

charlesh88 commented Nov 6, 2023

ozyx left a comment

charlesh88 commented Nov 7, 2023

ozyx left a comment

scottbell left a comment

ozyx left a comment

Gauge fixes for NaN and composition policy #5608

Gauge fixes for NaN and composition policy #5608

Conversation

charlesh88 commented Aug 4, 2022 • edited by ozyx Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

codecov bot commented Aug 4, 2022 • edited Loading

Codecov Report

scottbell left a comment

Choose a reason for hiding this comment

akhenry commented Oct 17, 2022

davetsay commented Jan 30, 2023

deploysentinel bot commented Oct 24, 2023 • edited Loading

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Test Case Results

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

charlesh88 commented Nov 6, 2023

ozyx left a comment

Choose a reason for hiding this comment

charlesh88 commented Nov 7, 2023

ozyx left a comment

Choose a reason for hiding this comment

scottbell left a comment

Choose a reason for hiding this comment

ozyx left a comment

Choose a reason for hiding this comment

charlesh88 commented Aug 4, 2022 •

edited by ozyx

Loading

codecov bot commented Aug 4, 2022 •

edited

Loading

deploysentinel bot commented Oct 24, 2023 •

edited

Loading