-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Fix flaky ServerMetricsCollector integration test #65420
Fix flaky ServerMetricsCollector integration test #65420
Conversation
Pinging @elastic/kibana-platform (Team:Platform) |
sendGet('/').end(() => null); | ||
const res1 = sendGet('/').then(res => res); |
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.
Supertest API... The request is sent when calling/awaiting .then
. I'm forced to noop-chain the promise to execute the request without awaiting it.
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.
How it's different from .end()
? I noticed that in case of end()
, it logs superagent request was sent twice, because both .end() and .then() were called. Never call .end() if you use promises
. Does it send a request twice 🤦
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.
Would you mind adding a comment about the usage of such a sophisticated construction in the test?
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.
Does it send a request twice
Yea...
const req = sendGet('/').end(() => null);
// later
await req;
Does send the request on each line... Imho having .then
executing the request is a very bad decision from this lib, but that's how it works... Which is why I had to use this .then
trick to be able to execute the request and still await for it later.
Will add a comment
💚 Build SucceededTo update your PR or re-run it, just comment with: |
* fix flaky test * add comment on supertest behavior
* master: (72 commits) add tsvb tests to Firefox suite (elastic#65425) Fix flaky ServerMetricsCollector integration test (elastic#65420) [APM] Custom links section inside the Actions menu is showing outside of the menu (elastic#65428) [ML] Adds docs_per_second to transform edit form. (elastic#65365) update apm index pattern (elastic#65424) add direct build command (elastic#65431) [ML] Adding daily_model_snapshot_retention_after_days to types and schemas (elastic#65417) [chore] Improve request cancelation handling in vis embeddable (elastic#65057) [Alerting] migrates acceptance and functional test fixtures to KP (elastic#64888) [ML] Fixes reordering in view by selection when overall cell selected (elastic#65290) Additional branding updates (elastic#64712) Remove redundant formatting of percentage column (elastic#64948) [SIEM][CASE] Configuration pages UI redesign (elastic#65355) New nav (elastic#64018) [Ingest pipelines] Address copy feedback (elastic#65175) bug fixing (elastic#65387) skip whole suite blocking snapshots (elastic#65377) add related event generation to ancestor nodes (fixes a bug) (elastic#64950) [Canvas] move files from legacy/plugins to plugins (elastic#65283) [SIEM] template timeline UI (elastic#64439) ...
* fix flaky test * add comment on supertest behavior
Fix #59236