[RAC] [Observability] Add functional tests covering the alert workflow status #111788
Conversation
weltenwort
added
v8.0.0
Team:Infra Monitoring UI - DEPRECATED
weltenwort
force-pushed
the
rac-test-workflow-actions
branch
from
536674f to
5126769
Compare
weltenwort
force-pushed
the
rac-test-workflow-actions
branch
from
5126769 to
b7ab54b
Compare
|
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
x-pack/test/observability_functional/apps/observability/alerts/workflow_status.ts
Outdated
Show resolved
Hide resolved
| await esArchiver.unload('x-pack/test/functional/es_archives/observability/alerts'); | ||
| }); | ||
|
|
||
| it('is filtered for "open" by default', async () => { |
There was a problem hiding this comment.
Nit: Is it more more common to say "filter by"? This confused my brain for a moment
There was a problem hiding this comment.
In "filter by" I'm always wondering whether it means to include or exclude. But I can probably try harder to find an unambiguous description with more natural grammar.
There was a problem hiding this comment.
I've tried to make the test case names clearer.
There was a problem hiding this comment.
FWIW for me, by always means include. Similar to how "sort by" works, you sort by what you desire, and you filter by what you desire. But the "predicate" nature of this makes it harder I agree.
But I would imagine it likes this: filter by $PREDICATE, where if the predicate is true you include it, so "open" vs "not open"
x-pack/test/observability_functional/apps/observability/alerts/workflow_status.ts
Show resolved
Hide resolved
x-pack/test/observability_functional/apps/observability/alerts/workflow_status.ts
Outdated
Show resolved
Hide resolved
|
Just came to review this, but @miltonhultgren has it covered 👌 I'll unassign myself. |
|
|
||
| const ACTION_COLUMN_INDEX = 1; | ||
|
|
||
| type WorkflowStatus = 'open' | 'acknowledged' | 'closed'; |
There was a problem hiding this comment.
Just out of curiosity, what is the risk of pulling this from the production code base?
Is it more from the perspective of blackbox testing or risk of coupling the code bases from a tooling perspective?
There was a problem hiding this comment.
The tooling can be problematic as well (TS performance in the test directories is terrible already), but the former point weighs heavier for me. These tests are supposed to be from the user's perspective and would therefore be aligned more with the UI concepts than the technical concepts. One example for that "looks the same, but actually isn't" is the fact that I needed to address the menu item via the verb "close" rather than the state "closed". So there is a correspondence between action and state, but they are not identical.
x-pack/test/observability_functional/apps/observability/alerts/workflow_status.ts
Show resolved
Hide resolved
| }); | ||
| }); | ||
|
|
||
| it('can be set to "acknowledged" using the row menu', async () => { |
There was a problem hiding this comment.
Nit: Similar here, this test name is still a bit unclear with regards to what "can be set". Maybe the word alert is needed here as well?
"an alert can be set to "acknowledged" using the row action menu"?
I don't think this needs to be changed, more looking to discuss patterns.
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @weltenwort |
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…w status (#111788) (#112421) Co-authored-by: Felix Stürmer <[email protected]>
📝 Summary
This adds functional tests for the alert workflow status actions in the alerts table's row actions.
closes #110641
🕵️♀️ Review notes
index.tsfile.