-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[APM] Fix some warnings logged in APM tests #52487
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
retest |
|
||
return () => { | ||
delayed.destroy(); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, much better. I wonder if the lack of proper cleanup caused bugs in the past?
@@ -76,7 +76,8 @@ describe('when simulating race condition', () => { | |||
|
|||
it('should render "Hello from Peter" after 200ms', async () => { | |||
jest.advanceTimersByTime(200); | |||
await tick(); | |||
|
|||
await wait(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍 Wouldn't mind if you removed tick
entirely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fixed now.
(Seemingly) since the React upgrade in 439708a, our tests have started logging various warnings/errors to the console. The test suite is still passing but it creates a lot of noise. Changes: - use `act` or `wait` when appropriate - mock useFetcher calls - cleanup in useDelayedVisbility
0ca2f3b
to
4fd10c7
Compare
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…le-sql-highlighting * 'master' of github.com:elastic/kibana: (56 commits) Migrate url shortener service (elastic#50896) Re-enable datemath in from/to canvas timelion args (elastic#52159) [Logs + Metrics UI] Remove eslint exceptions (elastic#50979) [Logs + Metrics UI] Add missing headers in Logs & metrics (elastic#52405) [ML] API integration tests - initial tests for bucket span estimator (elastic#52636) [Watcher] New Platform (NP) Migration (elastic#50908) Decouple Authorization subsystem from Legacy API. (elastic#52638) [APM] Fix some warnings logged in APM tests (elastic#52487) [ui/public/utils] Delete unused base_object & find_by_param (elastic#52500) [ui/public/utils] Move items into ui/vis (elastic#52615) fix newlines in kbn-analytics build script Add top level examples folder and command to run, `--run-examples`. (elastic#52027) feat(NA): add trap for SIGINT in the git precommit hook (elastic#52662) [DOCS] Updtes description of elasticsearch.requestHeadersWhitelist (elastic#52675) [Telemetry/Pulse] Updates advanced settings text for usage data (elastic#52657) [SIEM][Detection Engine] Adds the default name space to the end of the signals index [Logs UI] Generalize ML module management (elastic#50662) Removing stateful saved object finder (elastic#52166) Shim oss telemetry (elastic#51168) [Reporting/Screenshots] Do not fail the report if request is aborted (elastic#52344) ... # Conflicts: # src/legacy/core_plugins/console/public/legacy.ts # src/legacy/core_plugins/console/public/np_ready/application/models/legacy_core_editor/mode/elasticsearch_sql_highlight_rules.ts # src/legacy/core_plugins/console/public/np_ready/lib/autocomplete/components/full_request_component.ts # src/legacy/core_plugins/console/public/quarantined/src/sense_editor/row_parser.js
* [APM] Fix some warnings logged in APM tests (Seemingly) since the React upgrade in 439708a, our tests have started logging various warnings/errors to the console. The test suite is still passing but it creates a lot of noise. Changes: - use `act` or `wait` when appropriate - mock useFetcher calls - cleanup in useDelayedVisbility * Replace tick() with wait()
* [APM] Fix some warnings logged in APM tests (Seemingly) since the React upgrade in 439708a, our tests have started logging various warnings/errors to the console. The test suite is still passing but it creates a lot of noise. Changes: - use `act` or `wait` when appropriate - mock useFetcher calls - cleanup in useDelayedVisbility * Replace tick() with wait()
(Seemingly) since the React upgrade in 439708a, our tests have started logging various warnings/errors to the console. The test suite is still passing but it creates a lot of noise.
Changes:
act
orwait
when appropriate