Failing test: Jest Tests.src/plugins/dashboard/public/dashboard_container/embeddable/create - creates new embeddable with incoming embeddable if id does not match existing panel #155777
Assignees
Labels
failed-test
A test failure on a tracked branch, potentially flaky-test
impact:critical
This issue should be addressed immediately due to a critical level of impact on the product.
loe:medium
Medium Level of Effort
Team:Presentation
Presentation Team for Dashboard, Input Controls, and Canvas
Comments
kibanamachine
added
the
failed-test
kibanamachine
added
the
Team:Presentation
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
|
Seems like this might be a race condition caused by #155648 - looking into it 🖖 |
|
New failure: CI Build - main |
|
New failure: CI Build - main |
Heenawter
added a commit
that referenced
this issue
Apr 28, 2023
Closes #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](#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)
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this issue
Apr 28, 2023
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)
kibanamachine
referenced
this issue
Apr 28, 2023
# Backport This will backport the following commits from `main` to `8.8`: - [[Dashboard] Fix flaky Dashboard create tests (#156085)](#156085) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Hannah Mudge","email":"Heenawter@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-04-28T14:14:12Z","message":"[Dashboard] Fix flaky Dashboard create tests (#156085)\n\nCloses https://github.com/elastic/kibana/issues/155777\r\n\r\n## Summary\r\n\r\nThe `\"replaces panel with incoming embeddable if id matches existing\r\npanel\"` test was **not** the flaky one, even though it was the one that\r\nshowed the failure; it was actually one of the previous `\"pulls state\r\n<...>\"` tests that was failing, which was pretty confusing to catch 🤦\r\n\r\nBasically, because we are [running the unsaved changes check on load\r\nnow](#155648), depending on the\r\ntiming of the `debounce` on the `checkForUnsavedChangesSubject# Backport This will backport the following commits from `main` to `8.8`: - [[Dashboard] Fix flaky Dashboard create tests (#156085)](#156085) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT \r\nsubscription it would sometimes run for the `\"pulls state <...>\"` tests\r\n(but not always) - so whenever it **would** run, because the mocked\r\n`loadDashboardStateFromSavedObject` was only returning a partial\r\nDashboard input, this would result in trying to get the property of\r\n`undefined` when checking for filter changes, panel changes, etc.\r\n\r\nThis is fixed by ensuring that the `loadDashboardStateFromSavedObject`\r\nreturns a complete Dashboard input, with all the required keys, for all\r\nof the relevant tests. Note that, since you can't test Jest unit tests\r\nusing the flaky test runner, I was able to run it multiple times by\r\nsurrounding all the tests with the following in order to ensure that it\r\nwas no longer flaky:\r\n\r\n```typescript\r\nfor (const i of Array(x)\r\n .fill(null)\r\n .map((_, i) => i)) {\r\n describe(`test run ${i}`, () => {\r\n <...> // all the tests go here\r\n });\r\n};\r\n```\r\n\r\nI did this with `x = 200`, and the entire test suite passed 200 times in\r\na row 👍\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"57797731905b0303eec9059e398388826c88fa5f","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","loe:days","release_note:skip","impact:critical","v8.8.0","v8.9.0"],"number":156085,"url":"https://github.com/elastic/kibana/pull/156085","mergeCommit":{"message":"[Dashboard] Fix flaky Dashboard create tests (#156085)\n\nCloses https://github.com/elastic/kibana/issues/155777\r\n\r\n## Summary\r\n\r\nThe `\"replaces panel with incoming embeddable if id matches existing\r\npanel\"` test was **not** the flaky one, even though it was the one that\r\nshowed the failure; it was actually one of the previous `\"pulls state\r\n<...>\"` tests that was failing, which was pretty confusing to catch 🤦\r\n\r\nBasically, because we are [running the unsaved changes check on load\r\nnow](#155648), depending on the\r\ntiming of the `debounce` on the `checkForUnsavedChangesSubject# Backport This will backport the following commits from `main` to `8.8`: - [[Dashboard] Fix flaky Dashboard create tests (#156085)](#156085) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT \r\nsubscription it would sometimes run for the `\"pulls state <...>\"` tests\r\n(but not always) - so whenever it **would** run, because the mocked\r\n`loadDashboardStateFromSavedObject` was only returning a partial\r\nDashboard input, this would result in trying to get the property of\r\n`undefined` when checking for filter changes, panel changes, etc.\r\n\r\nThis is fixed by ensuring that the `loadDashboardStateFromSavedObject`\r\nreturns a complete Dashboard input, with all the required keys, for all\r\nof the relevant tests. Note that, since you can't test Jest unit tests\r\nusing the flaky test runner, I was able to run it multiple times by\r\nsurrounding all the tests with the following in order to ensure that it\r\nwas no longer flaky:\r\n\r\n```typescript\r\nfor (const i of Array(x)\r\n .fill(null)\r\n .map((_, i) => i)) {\r\n describe(`test run ${i}`, () => {\r\n <...> // all the tests go here\r\n });\r\n};\r\n```\r\n\r\nI did this with `x = 200`, and the entire test suite passed 200 times in\r\na row 👍\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"57797731905b0303eec9059e398388826c88fa5f"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156085","number":156085,"mergeCommit":{"message":"[Dashboard] Fix flaky Dashboard create tests (#156085)\n\nCloses https://github.com/elastic/kibana/issues/155777\r\n\r\n## Summary\r\n\r\nThe `\"replaces panel with incoming embeddable if id matches existing\r\npanel\"` test was **not** the flaky one, even though it was the one that\r\nshowed the failure; it was actually one of the previous `\"pulls state\r\n<...>\"` tests that was failing, which was pretty confusing to catch 🤦\r\n\r\nBasically, because we are [running the unsaved changes check on load\r\nnow](#155648), depending on the\r\ntiming of the `debounce` on the `checkForUnsavedChangesSubject# Backport This will backport the following commits from `main` to `8.8`: - [[Dashboard] Fix flaky Dashboard create tests (#156085)](#156085) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT \r\nsubscription it would sometimes run for the `\"pulls state <...>\"` tests\r\n(but not always) - so whenever it **would** run, because the mocked\r\n`loadDashboardStateFromSavedObject` was only returning a partial\r\nDashboard input, this would result in trying to get the property of\r\n`undefined` when checking for filter changes, panel changes, etc.\r\n\r\nThis is fixed by ensuring that the `loadDashboardStateFromSavedObject`\r\nreturns a complete Dashboard input, with all the required keys, for all\r\nof the relevant tests. Note that, since you can't test Jest unit tests\r\nusing the flaky test runner, I was able to run it multiple times by\r\nsurrounding all the tests with the following in order to ensure that it\r\nwas no longer flaky:\r\n\r\n```typescript\r\nfor (const i of Array(x)\r\n .fill(null)\r\n .map((_, i) => i)) {\r\n describe(`test run ${i}`, () => {\r\n <...> // all the tests go here\r\n });\r\n};\r\n```\r\n\r\nI did this with `x = 200`, and the entire test suite passed 200 times in\r\na row 👍\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"57797731905b0303eec9059e398388826c88fa5f"}}]}] BACKPORT--> Co-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com>
Heenawter
added
loe:medium
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
A test failed on a tracked branch
First failure: CI Build - main
The text was updated successfully, but these errors were encountered: