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

[Remote Clock] Wait for first tick and recalculate historical request bounds #5433

Merged
merged 26 commits into from
Jul 7, 2022

Conversation

ozyx
Copy link
Contributor

@ozyx ozyx commented Jul 5, 2022

Closes #5221

Describe your changes:

Adds a TelemetryRequestInterceptor. Block historical requests until the remote clock receives its first tick, and then use the interceptor to recalculate the request bounds.

Tests for TelemetryRequestInterceptor to be added in a future PR.

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?
  • Unit 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)

@ozyx ozyx requested a review from akhenry July 5, 2022 20:51
@ozyx
Copy link
Contributor Author

ozyx commented Jul 6, 2022

@unlikelyzero @akhenry :

A bunch of these imagery view tests are failing now because the added interceptor logic is adding more time, and Vue doesn't have the opportunity to update in time. If we add more await Vue.nextTick(), we can get these to pass (bleh).

Most of these actions are already covered by e2e tests, should we instead convert the failing tests to e2e tests and thus maintain the same coverage?

Copy link
Contributor

@akhenry akhenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking really good! thanks also for converting the TelemetryAPI to an ES6 module, and for the excellent jsdocs.

There are some minor changes that I think will improve this (and avoid potential unexpected regressions).

src/api/telemetry/TelemetryAPI.js Show resolved Hide resolved
src/api/telemetry/TelemetryAPI.js Outdated Show resolved Hide resolved
src/api/telemetry/TelemetryAPI.js Outdated Show resolved Hide resolved
src/api/telemetry/TelemetryAPI.js Outdated Show resolved Hide resolved
src/api/telemetry/TelemetryAPI.js Outdated Show resolved Hide resolved
src/api/telemetry/TelemetryAPI.js Show resolved Hide resolved
src/plugins/remoteClock/requestInterceptor.js Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #5433 (a09db30) into release/2.0.5 (a09db30) will not change coverage.
The diff coverage is n/a.

❗ Current head a09db30 differs from pull request most recent head 45afaed. Consider uploading reports for the commit 45afaed to get more accurate results

Impacted file tree graph

@@              Coverage Diff               @@
##           release/2.0.5    #5433   +/-   ##
==============================================
  Coverage          52.68%   52.68%           
==============================================
  Files                590      590           
  Lines              21761    21761           
  Branches            2041     2041           
==============================================
  Hits               11465    11465           
  Misses             10036    10036           
  Partials             260      260           
Flag Coverage Δ
e2e-ci 37.39% <0.00%> (ø)
unit 50.94% <0.00%> (ø)

Continue to review full report at Codecov.

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

ozyx added 2 commits July 6, 2022 17:39
- This is to avoid a timing issue with Vue renders within the unit tests
@ozyx ozyx requested review from unlikelyzero and akhenry July 7, 2022 01:01
src/api/telemetry/TelemetryAPI.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@unlikelyzero unlikelyzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e2e improvements look great!

If you have time before Andrew's review, you might want to put in a Remote Clock stub that captures this behavior as filed in the initial bug.

/e2e/tests/plugins/remoteClock

//boilerplate
test.describe('Remote Clock Tests', () => {
    test.fixme('Remote Clock accepts telemetry on initial load', async ({ page }) => {
    //addInitScript to with remote clock
    //Navigate directly to telemetry by URL OR by clicking time conductor
    };
}

@ozyx ozyx requested a review from akhenry July 7, 2022 20:05
@akhenry akhenry enabled auto-merge (squash) July 7, 2022 23:49
@akhenry akhenry merged commit 063df72 into release/2.0.5 Jul 7, 2022
@unlikelyzero unlikelyzero deleted the telemetry-request-interceptor branch July 7, 2022 23:55
@ozyx ozyx restored the telemetry-request-interceptor branch July 8, 2022 16:02
shefalijoshi added a commit that referenced this pull request Jul 14, 2022
* Bump d3-selection from 1.3.2 to 3.0.0

Bumps [d3-selection](https://github.com/d3/d3-selection) from 1.3.2 to 3.0.0.
- [Release notes](https://github.com/d3/d3-selection/releases)
- [Commits](d3/d3-selection@v1.3.2...v3.0.0)

---
updated-dependencies:
- dependency-name: d3-selection
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

* Remove snapshot

* Fix imagery filter slider drag in flexible layouts (#5326) (#5350)

* Dont' mutate a stacked plot unless its user initiated (#5357)

* Port grid icons and imagery test to release 2.0.5 from master (#5360)

* Port grid icons to release 2.0.5 from master

* Port imagery test to release/2.0.5

* Restrict timestrip composition to time based plots, plans and imagery (#5161)

* Restrict timestrip composition to time based plots, plans and imagery

* Adds unit tests for timeline composition policy

* Addresses review comments
Improves tests

* Reuse test objects

Co-authored-by: Jamie V <[email protected]>

* Include objectStyles reference to conditionSetIdentifier in imports (#5354)

* Include objectStyles reference to conditionSetIdentifier in imports

* Add tests for export

* Refactored some code and removed console log

* Remove workarounds for chrome 'scrollTop' issue (#5375)

* Fix naming of method (#5368)

* Imagery View does not discard old images when they fall out of bounds (#5351)

* change to using telemetry collection

* fix tests

* added more unit tests

* Cherrypicked commits (#5390)

Co-authored-by: unlikelyzero <[email protected]>

* [Timer] Update 3dot menu actions appropriately (#5387)

* Call `removeAllListeners()` after emit

* Manually show/hide actions if within a view

* remove sneaky `console.log()`

* Add Timer e2e test

* Add to comments

* Avoid hard waits in Timer e2e test

- Assert against timer view state instead of menu options

* Let's also test actions from the Timer view

* 5391 Add preview and drag support to Grand Search (#5394)

* add preview and drag actions

* added unit test, simplified remove action

* do not hide search results in preview mode when clicking outside search results

* add semantic aria labels to enable e2e tests

* readd preview

* add e2e test

* remove commented out url

* add percy snapshot and add search to ci

* make percy stuff work

* linting

* fix percy again

* move percy snapshots to a visual test

* added separate visual test and changed test to fixtures

* fix fixtures path

* addressing review comments

* 5361 tags not persisting locally (#5408)

* fixed typo

* remove unneeded lookup

* fix tags adding and deleting

* more reliable way to remove tags

* break tests up for parallel execution

* fixed notebook tagging test

* enable e2e tests

* made schedule index comment more clear and fix uppercase/lowercase issue

* address e2e changes

* add unit test to bump coverage

* fix typo

* need to check on annotation creation if provider exists or not

* added fixtures

* undo silly couchdb commit

* Plot progress bar fix for 2.0.5 (#5386)

* Add .bind(this) to stopLoading() in loadMoreData()

* Replace load spinner with progress bar for plots

* Add loading delay prop to swg

* fix linting errors

* match load order

* Update accessibility

* Add Math.max to timeout to handle negative inputs

* Moved math.max to load delay variable

* Add loading fix for stacked plots

* Move loadingUpdate func into plot item for update

* Merge conflict resolve

* Check if delay is 0 and send, put post in a func

* Put obj directly to model, removed computed prop

* Lint fix

* Fix template where legend was not displayed

* Remove commented out template

* Fixed failing test

Co-authored-by: unlikelyzero <[email protected]>

* Make plans non editable. (#5377)

* Make plans non editable.

* Add unit test for fix

* [CouchDB] Better determination of indicator status (#5415)

* Add unknown state, remove maintenance state

* Handle all CouchDB status codes

- Set unknown status if we receive an unhandled code

* Include status code in error messages

* SharedWorker can send unknown status

* Add test for unknown status

* Gauge fixes for Firefox and units display (#5369)

* Closes #5323, #5325. Parent branch is release/2.0.5.
- Significant work refactoring SVG markup and CSS for dial gauge;
- Fixed missing `v-if` to control display of units for #5325;
- Fixed bad `.length` test for limit properties;

* Closes #5323, #5325
- Add 'value out of range' indicator

* Closes #5323, #5325
- More accurate element naming;
- Fix cross-browser problems with current value display in dial gauge;
- Refinements to "out of range" indicator approach;
- Fixed size of "Amplitude" input in Sine Wave Generator;

* Closes #5323, #5325
- Styles and stubbed in code to support needle meter type;

* Closes #5323, #5325
- Stubbed in markup and CSS for needle-style meter;

* Closes #5323, #5325
- Fixed missing `js-*` classes that were failing npm run test;

* Closes #5323, #5325
- Fix to not display meter value bar unless a data value is expected;

* Addressing PR comments
- Renamed method for clarity;
- Added null value check in method `valueExpected`;

* [Static Root] Return leafValue if null/undefined/false (#5416)

* Return leafValue if null/undefined/false

* Added a null to the test json

* Show a better default poll question (#5425)

* 5361 Tags not persisting when several notebook entries are created at once (#5428)

* add end to end test to catch multiple entry errors

* click expansion triangle instead

* fix race condition between annotation creation and mutation

* make sure notebook tags run in e2e

* address PR comments

* Handle missing objects gracefully  (#5399)

* Handle missing object errors for display layouts
* Handle missing object errors for Overlay Plots
* Add check for this.config
* Add try/catch statement & check if obj is missing
* Changed console.error to console.warn
* Lint fix
* Fix for this.metadata.value is undefined
* Add e2e test
* Update comment text
* Add reload check and @Private, verify console.warn
* Redid assignment and metadata check
* Fix typo
* Changed assignment and metadata check
* Redid checks for isMissing(object)
* Lint fix

* Backmerge e2e code coverage changes and fixes into release/2.0.5 (#5431)

* [Telemetry Collections] Respect "Latest" Strategy Option (#5421)

* Respect latest strategy in Telemetry Collections to limit potential memory growth.

* fix sourcemaps (#5373)

Co-authored-by: John Hill <[email protected]>

* Debounce status summary (#5448)

Co-authored-by: John Hill <[email protected]>

* No gauge (#5451)

* Installed gauge plugin by default
* Make gauge part of standard install in e2e suite and add restrictednotebook

Co-authored-by: Andrew Henry <[email protected]>

* [CouchDB] Always subscribe to the CouchDB changes feed (#5434)

* Add unknown state, remove maintenance state

* Handle all CouchDB status codes

- Set unknown status if we receive an unhandled code

* Include status code in error messages

* SharedWorker can send unknown status

* Add test for unknown status

* Always subscribe to CouchDB changes feed

- Always subscribe to the CouchDB changes feed, even if there are no observable objects, since we are also checking the status of CouchDB via this feed.

* Update indicator status if not using SharedWorker

* Start listening to changes feed on first request

* fix test

* adjust test to hopefully avoid race condition

* lint

Co-authored-by: John Hill <[email protected]>
Co-authored-by: Andrew Henry <[email protected]>
Co-authored-by: Scott Bell <[email protected]>

* Fix for Fault Management Visual Bugs  (#5376)

* Closes #5365
* General visual improvements

Co-authored-by: Charles Hacskaylo <[email protected]>
Co-authored-by: Andrew Henry <[email protected]>

* fix pathing (#5452)

Co-authored-by: Jesse Mazzella <[email protected]>

* [Static Root] Static Root Plugin not loading (#5455)

* Log if hitting falsy leafValue

* Add some logging

* Remove logs and specify null/undefined

Co-authored-by: Jesse Mazzella <[email protected]>

* Allow endpoints with a single enum metadata value in Bar/Line graphs (#5443)

* If there is only 1 metadata value, set yKey to none. Also, fix bug for determining the name of a metadata value
* Update tests for enum metadata values

Co-authored-by: John Hill <[email protected]>
Co-authored-by: Andrew Henry <[email protected]>

* [Remote Clock] Wait for first tick and recalculate historical request bounds (#5433)

* Updated to ES6 class
* added request intercept functionality to telemetry api, added a request interceptor for remote clock
* add remoteClock e2e test stub

Co-authored-by: Jamie V <[email protected]>
Co-authored-by: Andrew Henry <[email protected]>

* Fix for missing object for LADTableSet (#5458)

* Handle missing object errors for display layouts

Co-authored-by: Andrew Henry <[email protected]>

* removing the call for default import now that TelemetryAPI is an ES6 class (#5461)

* [Remote Clock] Fix requestInterceptor typo (#5462)

* Fix typo in telemetry request interceptor

Co-authored-by: Jamie V <[email protected]>
Co-authored-by: Andrew Henry <[email protected]>

* Lock model (#5457)

* Lock event Model to prevent reactification

* de-reactify all the things

* Make API properties writable to allow test mocks to override them

* Fix merge conflict

* Added plot interceptor for missing series config (#5422)

Co-authored-by: Andrew Henry <[email protected]>
Co-authored-by: Shefali Joshi <[email protected]>

* Remove performance marks (#5465)

* Remove performance marks

* Retain performance mark in view large. It doesn't happen very often and it's needed for an automated performance test

* Use timeKey for time comparison (#5471)

* Fix couchdb no response (#5474)

* Update the creation date only when the document is created for the first time

* If there is no response from a bulk get, couch db has issues

* Check the response - if it's null, don't apply interceptors

* Fix shelved alarms (#5479)

* Fix the logic around shelved alarms

* Remove application router listener

* Release 2.0.5 UI and Gauge fixes (#5470)

* Various UI fixes
- Tweak to Gauge properties form for clarity and usability.
- Fix Gauge 'dial' type not obeying "Show units" property setting, closes #5325.
- Tweaks to Operator Status UI label and layout for clarity.
- Changed name and description of Graph object for clarity and consistency.
- Fixed CSS classing that was coloring Export menu items text incorrectly.
- Fixed icon-to-text vertical alignment in `.c-object-label`.
- Fix for broken layout in imagery local controls (brightness, layers, magnification).

Co-authored-by: Andrew Henry <[email protected]>

* Stacked plot interceptor rename (#5468)

* Rename stacked plot interceptor and move to folder

Co-authored-by: Andrew Henry <[email protected]>

* Clear data when time bounds are changed (#5482)

* Clear data when time bounds are changed
Also react to clear data action
Ensure that the yKey is set to 'none' if there is no range with array Values

* Refactor trace updates to a method

* get rid of root (#5483)

* Do not pass onPartialResponse option on to upstream telemetry (#5486)

* Fix all of the e2e tests (#5477)

* Fix timer test

* be explicit about the warnings text

* add full suite to CI to enable CircleCI Checks

* add back in devtool=false for CI env so firefox tests run

* add framework suite

* Don't install webpack HMR in CI

* Fix playwright version installs

* exclude HMR if running tests in any environment

- use NODE_ENV=TEST to exclude webpack HMR

- deparameterize some of the playwright configs

* use lower-case 'test'

* timer hover fix

* conditionally skip for firefox due to missing console events

* increase timeouts to give time for mutation

* no need to close save banner

* remove devtool setting

* revert

* update snapshots

* disable video to save some resources

* use one worker

* more timeouts :)

* Remove `browser.close()` and `page.close()` as it was breaking other tests

* Remove unnecessary awaits and fix func call syntax

* Fix image reset test

* fix restrictedNotebook tests

* revert playwright-ci.config settings

* increase timeout for polling imagery test

* remove unnecessary waits

* disable notebook lock test for chrome-beta as its unreliable

- remove some unnecessary 'wait for save banner' logic

- remove unused await

- mark imagery test as slow in chrome-beta

* LINT!! *shakes fist*

* don't run full e2e suite per commit

* disable video in all configs

* add flakey zoom comment

* exclude webpack HMR in non-development modes

Co-authored-by: Jesse Mazzella <[email protected]>
Co-authored-by: Jesse Mazzella <[email protected]>

* lint fix

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joshi <[email protected]>
Co-authored-by: Jesse Mazzella <[email protected]>
Co-authored-by: Jamie V <[email protected]>
Co-authored-by: Andrew Henry <[email protected]>
Co-authored-by: Scott Bell <[email protected]>
Co-authored-by: unlikelyzero <[email protected]>
Co-authored-by: Alize Nguyen <[email protected]>
Co-authored-by: Charles Hacskaylo <[email protected]>
Co-authored-by: Khalid Adil <[email protected]>
Co-authored-by: rukmini-bose <[email protected]>
Co-authored-by: Jesse Mazzella <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants