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

[8.8] [Dashboard] Fix flaky Dashboard create tests (#156085) #156180

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

kibanamachine
Copy link
Contributor

Backport

This will backport the following commits from main to 8.8:

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Closes elastic#155777

## Summary

The `"replaces panel with incoming embeddable if id matches existing
panel"` test was **not** the flaky one, even though it was the one that
showed the failure; it was actually one of the previous `"pulls state
<...>"` tests that was failing, which was pretty confusing to catch 🤦

Basically, because we are [running the unsaved changes check on load
now](elastic#155648), depending on the
timing of the `debounce` on the `checkForUnsavedChangesSubject$`
subscription it would sometimes run for the `"pulls state <...>"` tests
(but not always) - so whenever it **would** run, because the mocked
`loadDashboardStateFromSavedObject` was only returning a partial
Dashboard input, this would result in trying to get the property of
`undefined` when checking for filter changes, panel changes, etc.

This is fixed by ensuring that the `loadDashboardStateFromSavedObject`
returns a complete Dashboard input, with all the required keys, for all
of the relevant tests. Note that, since you can't test Jest unit tests
using the flaky test runner, I was able to run it multiple times by
surrounding all the tests with the following in order to ensure that it
was no longer flaky:

```typescript
for (const i of Array(x)
  .fill(null)
  .map((_, i) => i)) {
  describe(`test run ${i}`, () => {
    <...> // all the tests go here
  });
};
```

I did this with `x = 200`, and the entire test suite passed 200 times in
a row 👍

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 5779773)
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 17 19 +2
securitySolution 399 402 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 18 20 +2
securitySolution 479 482 +3
total +5

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @Heenawter

@kibanamachine kibanamachine merged commit e2c108c into elastic:8.8 Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants