Failing test: Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/dashboard/group2/panel_titles·ts - dashboard panel titles by reference unlinking a by reference panel with a custom title will keep the current title #156539
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
|
New failure: CI Build - main |
dej611
added
the
Team:Presentation
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
Heenawter
added
the
impact:critical
|
New failure: CI Build - main |
|
Skipped. main: b7bd376 |
|
New failure: CI Build - main |
|
New failure: CI Build - main |
3 tasks
Heenawter
added a commit
that referenced
this issue
May 5, 2023
Closes #156539 Closes #156544 ## Summary As part of investigating the attached flaky test suite, I realized that the flakiness was because we weren't waiting long enough for a panel to be added and/or removed from the library - so, I added a check to ensure the library notification appears in `saveToLibrary`, and similarly I added a check to ensure that the library notification **disappears** in `unlinkFromLibary`. However, after adding these extra checks, the following test started to fail: https://github.com/elastic/kibana/blob/23a45bde21b36be49a1174b9bde1fc340de67534/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts#L148-L155 Way, way, way back in `8.1`, one of my [first PRs](#120815) was meant to fix some problems with dashboard panel titles - as part of this, I was **supposed** to make sure that, if a by-reference panel is given a custom panel title, the title should remain the same after unlinking (i.e. it should remain as the custom title rather than resetting to the by-reference title). So, I added the above test to verify this behaviour. Turns out, though, that this test had a flaw - because we weren't waiting long enough for the panel to actually be disconnected from the library, this test was only passing because it was grabbing and comparing titles **before** the unlink was complete - so, even though the title actually **was** being reset back to the original library title, this test did not catch this bug. This has been the case since `8.1` when this test was introduced - not sure how it was missed, but we never actually fixed the bug where dashboard panel titles are getting reset when unlinking from the library. So, this PR actually accomplishes two things: 1) It fixes the flakiness of the attached tests by adding the extra library notification checks: <a href="https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2217"><img src="https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png"/></a> 2) It ensures that, if a by-reference panel has a custom title, unlinking it from the library will not impact that title. ### Before https://user-images.githubusercontent.com/8698078/236062484-b1cedc47-cb19-4273-aec7-17d24b55953b.mov ### After https://user-images.githubusercontent.com/8698078/236062862-aace4f09-d007-401a-ba57-b9b74fda0b33.mov ### 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 - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### 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
May 5, 2023
Closes elastic#156539 Closes elastic#156544 ## Summary As part of investigating the attached flaky test suite, I realized that the flakiness was because we weren't waiting long enough for a panel to be added and/or removed from the library - so, I added a check to ensure the library notification appears in `saveToLibrary`, and similarly I added a check to ensure that the library notification **disappears** in `unlinkFromLibary`. However, after adding these extra checks, the following test started to fail: https://github.com/elastic/kibana/blob/23a45bde21b36be49a1174b9bde1fc340de67534/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts#L148-L155 Way, way, way back in `8.1`, one of my [first PRs](elastic#120815) was meant to fix some problems with dashboard panel titles - as part of this, I was **supposed** to make sure that, if a by-reference panel is given a custom panel title, the title should remain the same after unlinking (i.e. it should remain as the custom title rather than resetting to the by-reference title). So, I added the above test to verify this behaviour. Turns out, though, that this test had a flaw - because we weren't waiting long enough for the panel to actually be disconnected from the library, this test was only passing because it was grabbing and comparing titles **before** the unlink was complete - so, even though the title actually **was** being reset back to the original library title, this test did not catch this bug. This has been the case since `8.1` when this test was introduced - not sure how it was missed, but we never actually fixed the bug where dashboard panel titles are getting reset when unlinking from the library. So, this PR actually accomplishes two things: 1) It fixes the flakiness of the attached tests by adding the extra library notification checks: <a href="https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2217"><img src="https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png"/></a> 2) It ensures that, if a by-reference panel has a custom title, unlinking it from the library will not impact that title. ### Before https://user-images.githubusercontent.com/8698078/236062484-b1cedc47-cb19-4273-aec7-17d24b55953b.mov ### After https://user-images.githubusercontent.com/8698078/236062862-aace4f09-d007-401a-ba57-b9b74fda0b33.mov ### 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 - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### 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 ce7ef40)
kibanamachine
referenced
this issue
May 5, 2023
…#156871) # Backport This will backport the following commits from `main` to `8.8`: - [[Dashboard] Fix "Unlink from library" panel title bug (#156589)](#156589) <!--- 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-05-05T14:54:39Z","message":"[Dashboard] Fix \"Unlink from library\" panel title bug (#156589)\n\nCloses https://github.com/elastic/kibana/issues/156539\r\nCloses https://github.com/elastic/kibana/issues/156544\r\n\r\n## Summary\r\n\r\n\r\nAs part of investigating the attached flaky test suite, I realized that\r\nthe flakiness was because we weren't waiting long enough for a panel to\r\nbe added and/or removed from the library - so, I added a check to ensure\r\nthe library notification appears in `saveToLibrary`, and similarly I\r\nadded a check to ensure that the library notification **disappears** in\r\n`unlinkFromLibary`.\r\n\r\nHowever, after adding these extra checks, the following test started to\r\nfail:\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/23a45bde21b36be49a1174b9bde1fc340de67534/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts#L148-L155\r\n\r\nWay, way, way back in `8.1`, one of my [first\r\nPRs](#120815) was meant to fix\r\nsome problems with dashboard panel titles - as part of this, I was\r\n**supposed** to make sure that, if a by-reference panel is given a\r\ncustom panel title, the title should remain the same after unlinking\r\n(i.e. it should remain as the custom title rather than resetting to the\r\nby-reference title). So, I added the above test to verify this\r\nbehaviour.\r\n\r\nTurns out, though, that this test had a flaw - because we weren't\r\nwaiting long enough for the panel to actually be disconnected from the\r\nlibrary, this test was only passing because it was grabbing and\r\ncomparing titles **before** the unlink was complete - so, even though\r\nthe title actually **was** being reset back to the original library\r\ntitle, this test did not catch this bug. This has been the case since\r\n`8.1` when this test was introduced - not sure how it was missed, but we\r\nnever actually fixed the bug where dashboard panel titles are getting\r\nreset when unlinking from the library.\r\n\r\nSo, this PR actually accomplishes two things:\r\n1) It fixes the flakiness of the attached tests by adding the extra\r\nlibrary notification checks:\r\n<a\r\nhref=\"https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2217\"><img\r\nsrc=\"https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png\"/></a>\r\n2) It ensures that, if a by-reference panel has a custom title,\r\nunlinking it from the library will not impact that title.\r\n\r\n### Before\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/236062484-b1cedc47-cb19-4273-aec7-17d24b55953b.mov\r\n\r\n\r\n### After\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/236062862-aace4f09-d007-401a-ba57-b9b74fda0b33.mov\r\n\r\n\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- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\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":"ce7ef40d9a79313be64057af6fb7f143a44a434a","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Feature:Embedding","release_note:fix","Team:Presentation","loe:days","impact:high","backport:prev-minor","v8.9.0"],"number":156589,"url":"https://github.com/elastic/kibana/pull/156589","mergeCommit":{"message":"[Dashboard] Fix \"Unlink from library\" panel title bug (#156589)\n\nCloses https://github.com/elastic/kibana/issues/156539\r\nCloses https://github.com/elastic/kibana/issues/156544\r\n\r\n## Summary\r\n\r\n\r\nAs part of investigating the attached flaky test suite, I realized that\r\nthe flakiness was because we weren't waiting long enough for a panel to\r\nbe added and/or removed from the library - so, I added a check to ensure\r\nthe library notification appears in `saveToLibrary`, and similarly I\r\nadded a check to ensure that the library notification **disappears** in\r\n`unlinkFromLibary`.\r\n\r\nHowever, after adding these extra checks, the following test started to\r\nfail:\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/23a45bde21b36be49a1174b9bde1fc340de67534/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts#L148-L155\r\n\r\nWay, way, way back in `8.1`, one of my [first\r\nPRs](#120815) was meant to fix\r\nsome problems with dashboard panel titles - as part of this, I was\r\n**supposed** to make sure that, if a by-reference panel is given a\r\ncustom panel title, the title should remain the same after unlinking\r\n(i.e. it should remain as the custom title rather than resetting to the\r\nby-reference title). So, I added the above test to verify this\r\nbehaviour.\r\n\r\nTurns out, though, that this test had a flaw - because we weren't\r\nwaiting long enough for the panel to actually be disconnected from the\r\nlibrary, this test was only passing because it was grabbing and\r\ncomparing titles **before** the unlink was complete - so, even though\r\nthe title actually **was** being reset back to the original library\r\ntitle, this test did not catch this bug. This has been the case since\r\n`8.1` when this test was introduced - not sure how it was missed, but we\r\nnever actually fixed the bug where dashboard panel titles are getting\r\nreset when unlinking from the library.\r\n\r\nSo, this PR actually accomplishes two things:\r\n1) It fixes the flakiness of the attached tests by adding the extra\r\nlibrary notification checks:\r\n<a\r\nhref=\"https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2217\"><img\r\nsrc=\"https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png\"/></a>\r\n2) It ensures that, if a by-reference panel has a custom title,\r\nunlinking it from the library will not impact that title.\r\n\r\n### Before\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/236062484-b1cedc47-cb19-4273-aec7-17d24b55953b.mov\r\n\r\n\r\n### After\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/236062862-aace4f09-d007-401a-ba57-b9b74fda0b33.mov\r\n\r\n\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- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\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":"ce7ef40d9a79313be64057af6fb7f143a44a434a"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156589","number":156589,"mergeCommit":{"message":"[Dashboard] Fix \"Unlink from library\" panel title bug (#156589)\n\nCloses https://github.com/elastic/kibana/issues/156539\r\nCloses https://github.com/elastic/kibana/issues/156544\r\n\r\n## Summary\r\n\r\n\r\nAs part of investigating the attached flaky test suite, I realized that\r\nthe flakiness was because we weren't waiting long enough for a panel to\r\nbe added and/or removed from the library - so, I added a check to ensure\r\nthe library notification appears in `saveToLibrary`, and similarly I\r\nadded a check to ensure that the library notification **disappears** in\r\n`unlinkFromLibary`.\r\n\r\nHowever, after adding these extra checks, the following test started to\r\nfail:\r\n\r\n\r\nhttps://github.com/elastic/kibana/blob/23a45bde21b36be49a1174b9bde1fc340de67534/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts#L148-L155\r\n\r\nWay, way, way back in `8.1`, one of my [first\r\nPRs](#120815) was meant to fix\r\nsome problems with dashboard panel titles - as part of this, I was\r\n**supposed** to make sure that, if a by-reference panel is given a\r\ncustom panel title, the title should remain the same after unlinking\r\n(i.e. it should remain as the custom title rather than resetting to the\r\nby-reference title). So, I added the above test to verify this\r\nbehaviour.\r\n\r\nTurns out, though, that this test had a flaw - because we weren't\r\nwaiting long enough for the panel to actually be disconnected from the\r\nlibrary, this test was only passing because it was grabbing and\r\ncomparing titles **before** the unlink was complete - so, even though\r\nthe title actually **was** being reset back to the original library\r\ntitle, this test did not catch this bug. This has been the case since\r\n`8.1` when this test was introduced - not sure how it was missed, but we\r\nnever actually fixed the bug where dashboard panel titles are getting\r\nreset when unlinking from the library.\r\n\r\nSo, this PR actually accomplishes two things:\r\n1) It fixes the flakiness of the attached tests by adding the extra\r\nlibrary notification checks:\r\n<a\r\nhref=\"https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2217\"><img\r\nsrc=\"https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png\"/></a>\r\n2) It ensures that, if a by-reference panel has a custom title,\r\nunlinking it from the library will not impact that title.\r\n\r\n### Before\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/236062484-b1cedc47-cb19-4273-aec7-17d24b55953b.mov\r\n\r\n\r\n### After\r\n\r\n\r\nhttps://user-images.githubusercontent.com/8698078/236062862-aace4f09-d007-401a-ba57-b9b74fda0b33.mov\r\n\r\n\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- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\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":"ce7ef40d9a79313be64057af6fb7f143a44a434a"}}]}] BACKPORT--> Co-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com>
smith
pushed a commit
to smith/kibana
that referenced
this issue
May 5, 2023
![https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png"/></a](https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png"/></a){kind=link}
![https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2217\"><img\r\nsrc=\"https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png\"/></a>\r\n2](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2217\"><img\r\nsrc=\"https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png\"/></a>\r\n2){kind=link}
Closes elastic#156539 Closes elastic#156544 ## Summary As part of investigating the attached flaky test suite, I realized that the flakiness was because we weren't waiting long enough for a panel to be added and/or removed from the library - so, I added a check to ensure the library notification appears in `saveToLibrary`, and similarly I added a check to ensure that the library notification **disappears** in `unlinkFromLibary`. However, after adding these extra checks, the following test started to fail: https://github.com/elastic/kibana/blob/23a45bde21b36be49a1174b9bde1fc340de67534/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts#L148-L155 Way, way, way back in `8.1`, one of my [first PRs](elastic#120815) was meant to fix some problems with dashboard panel titles - as part of this, I was **supposed** to make sure that, if a by-reference panel is given a custom panel title, the title should remain the same after unlinking (i.e. it should remain as the custom title rather than resetting to the by-reference title). So, I added the above test to verify this behaviour. Turns out, though, that this test had a flaw - because we weren't waiting long enough for the panel to actually be disconnected from the library, this test was only passing because it was grabbing and comparing titles **before** the unlink was complete - so, even though the title actually **was** being reset back to the original library title, this test did not catch this bug. This has been the case since `8.1` when this test was introduced - not sure how it was missed, but we never actually fixed the bug where dashboard panel titles are getting reset when unlinking from the library. So, this PR actually accomplishes two things: 1) It fixes the flakiness of the attached tests by adding the extra library notification checks: <a href="https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2217"><img src="https://user-images.githubusercontent.com/8698078/236044233-8a07aadc-8a55-40f2-9ed2-798a91501f68.png"/></a> 2) It ensures that, if a by-reference panel has a custom title, unlinking it from the library will not impact that title. ### Before https://user-images.githubusercontent.com/8698078/236062484-b1cedc47-cb19-4273-aec7-17d24b55953b.mov ### After https://user-images.githubusercontent.com/8698078/236062862-aace4f09-d007-401a-ba57-b9b74fda0b33.mov ### 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 - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### 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)
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: