[ResponseOps][Rules] Fix rules list item enabled state updates causing test flakyness #183805
Conversation
umbopepato
force-pushed
the
157623-rules-list-flaky-test
branch
from
1af9586
to
4fbed12
Compare
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6038[✅] x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/config.ts: 50/50 tests passed. |
umbopepato
force-pushed
the
157623-rules-list-flaky-test
branch
from
4fbed12
to
925ed14
Compare
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6039[✅] x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/config.ts: 50/50 tests passed. |
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6040[✅] x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/config.ts: 100/100 tests passed. |
Flaky Test Runner Stats🟠 Some tests failed. - kibana-flaky-test-suite-runner#6041[❌] x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/config.ts: 174/200 tests passed. |
umbopepato
force-pushed
the
157623-rules-list-flaky-test
branch
from
925ed14
to
a9adc01
Compare
umbopepato
changed the title
|
/ci |
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6046[✅] x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/config.ts: 50/50 tests passed. |
umbopepato
added
release_note:skip
|
Pinging @elastic/response-ops (Team:ResponseOps) |
| @@ -60,7 +60,6 @@ export const RuleStatusDropdown: React.FunctionComponent<ComponentOpts> = ({ | |||
| hideSnoozeOption = false, | |||
| direction = 'column', | |||
| }: ComponentOpts) => { | |||
| const [isEnabled, setIsEnabled] = useState<boolean>(rule.enabled); | |||
| const [isSnoozed, setIsSnoozed] = useState<boolean>(!hideSnoozeOption && isRuleSnoozed(rule)); | |||
There was a problem hiding this comment.
nit: Could isSnoozed be also removed from useEffect and handled same way as isEnabled?
There was a problem hiding this comment.
I agree with Janki! We can derive the state from the props when the component is rendered.
There was a problem hiding this comment.
Yep, I quickly went through the setIsSnoozed usages and concluded it wasn't necessary since the data flow is unidirectional from rule, but a useMemo would be much better at handling this, commit coming in a few minutes 🙂
There was a problem hiding this comment.
Not even a useMemo 🙂. const isSnoozed = !hideSnoozeOption && isRuleSnoozed(rule) is more than enough!
umbopepato
force-pushed
the
157623-rules-list-flaky-test
branch
from
594383d
to
15ee943
Compare
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
kibanamachine
added
v8.15.0
backport:skip
Summary
Removes optimistic updates from rules list item when enabling/disabling rules to avoid out-of-sync states (the likely cause of this flaky test).
Fixes #157623