[Event Log] Ensure sorting tests are less flaky #64781
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gmmorris
added
Feature:Alerting
release_note:skip
|
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
spalger
reviewed
Apr 29, 2020
x-pack/test/plugin_api_integration/test_suites/event_log/public_api_integration.ts
Outdated
Show resolved
Hide resolved
|
FYI, I just skipped this suite so please make sure to unskip the tests in this PR. |
|
Also, not sure if this addresses #64812 too |
It does not, but the same fix here, to that code, will presumably fix that issue as well. Good to see that's all we needed to fix this! |
pmuellr
approved these changes
Apr 29, 2020
* master: (42 commits) [Ingest] Allow aggent to send metadata compliant with ECS (elastic#64452) [Endpoint] Remove todos, urls to issues (elastic#64833) [Uptime] Remove hard coded value for monitor states histograms (elastic#64396) Feature/send feedback link (elastic#64845) [ML] Moving get filters capability to admin (elastic#64879) Remove edit alert button from alerts list (elastic#64643) [EPM] Handle constant_keyword type in KB index patterns and ES index templates (elastic#64876) [ML] Disable data frame anaylics clone button based on permission (elastic#64830) Dashboard url generator to preserve saved filters from destination dashboard (elastic#64767) add generic typings for SavedObjectMigrationFn (elastic#63943) Allow to define and update a defaultPath for applications (elastic#64498) [Event Log] add rel=primary to saved objects for query targets (elastic#64615) [Lens] Use a size of 5 for first string field in visualization (elastic#64726) [SIEM][Lists] Removes plugin dependencies, adds more unit tests, fixes more TypeScript types [Ingest] Edit datasource UI (elastic#64727) [Lens] Bind all time fields to the time picker (elastic#63874) [Lens] Use suggestion system in chart switcher for subtypes (elastic#64613) Improve alpha messaging (elastic#64692) [Ingest] Allow to enable monitoring of elastic agent (elastic#63598) [Metrics UI] Fix alerting when a filter query is present (elastic#64575) ...
pmuellr
approved these changes
Apr 30, 2020
| @@ -42,6 +42,9 @@ export const logEventRoute = (router: IRouter, eventLogger: IEventLogger, logger | |||
| await context.core.savedObjects.client.create('event_log_test', {}, { id }); | |||
| logger.info(`created saved object ${id}`); | |||
| } | |||
| // mark now as start and end | |||
There was a problem hiding this comment.
The only slightly odd thing here is that for the test where we search by end date, all of these events will have an end date - however, IRL, not all events have end dates. We may need to do something in the future - like when searching via end date, add an additional filter to only return events with an end date - but let's wait to see how people use this, to see if something needs to be done.
There was a problem hiding this comment.
Yeah, agreed, though this is sort by end rather than search by end.. semantics but it means that the behaviour is inline with how ES queries generally behave.
|
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
May 4, 2020
Creating events in parallel may be causing a slight flakyness, this change staggers creation to ensure this doesn't happen. In addition it turned out the `event.end` field was missing in certain cases, causing the test that sorts by `end` to fail.
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
May 4, 2020
…ana into alerting/np-tests-migration * 'alerting/np-tests-migration' of github.com:gmmorris/kibana: [APM] Agent remote config: validation for Java agent configs (elastic#63956) [APM] Fix duplicate index patterns (elastic#64883) Drilldowns (elastic#61219) [Alerting] fix labels and links in PagerDuty action ui and docs (elastic#64032) [Event Log] Ensure sorting tests are less flaky (elastic#64781) update endpoint to restrict removing with datasources (elastic#64978) [Logs UI] [Alerting] Alerts management page enhancements (elastic#64654) Adjust kibana app owning files (elastic#65064) Migrate tutorial resources (elastic#64298) [Logs UI] Tweak copy in log alerts dialog (elastic#64645) [Logs UI] [Alerting] Documentation (elastic#64886) [Logs UI] Add dataset filter to ML module setup screen (elastic#64470) [TSVB] Fixing memory leak (elastic#64918) Bump backport to 5.4.1 (elastic#65041)
gmmorris
added a commit
that referenced
this pull request
May 5, 2020
Creating events in parallel may be causing a slight flakyness, this change staggers creation to ensure this doesn't happen. In addition it turned out the `event.end` field was missing in certain cases, causing the test that sorts by `end` to fail. Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Creating events in parallel may be causing a slight flakyness, this change staggers creation to ensure this doesn't happen.
In addition it turned out the
event.endfield was missing in certain cases, causing the test that sorts byendto fail.closes #64723 and #64812
Checklist
Delete any items that are not applicable to this PR.
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsThis was checked for keyboard-only and screenreader accessibilityThis renders correctly on smaller devices using a responsive layout. (You can test this in your browserThis was checked for cross-browser compatibility, including a check against IE11For maintainers