[ILM] Split edit policy test helpers into separate files#100807
[ILM] Split edit policy test helpers into separate files#100807yuliacech merged 5 commits intoelastic:masterfrom
Conversation
yuliacech
added
release_note:skip
|
@elasticmachine merge upstream |
| * | ||
| * @param switchTestSubject The test subject of the EuiSwitch (can be a nested path. e.g. "myForm.mySwitch"). | ||
| */ | ||
| toggleEuiSwitch: (switchTestSubject: T, isChecked?: boolean) => void; |
There was a problem hiding this comment.
The boolean was not used in the implementation of the toggle method.
|
|
||
| let button = find('disableDeletePhaseButton'); | ||
| if (!button.length) { | ||
| button = find('enableDeletePhaseButton'); |
There was a problem hiding this comment.
A button to disable the delete phase is only shown, when the delete phase is enabled.
|
@elasticmachine merge upstream |
| hasColdPhase: () => exists('ilmTimelineColdPhase'), | ||
| hasFrozenPhase: () => exists('ilmTimelineFrozenPhase'), | ||
| hasDeletePhase: () => exists('ilmTimelineDeletePhase'), | ||
| hasPhase: (phase: Phase) => exists(`ilmTimelinePhase-${phase}`), |
sabarasaba
left a comment
sabarasaba
left a comment
There was a problem hiding this comment.
Hey @yuliacech, thanks for working on this! It looks really good! I ran everything locally, works flawlessly, and sooo much more readable now. Just one no- blocking comment that we can totally ignore for now:
| test(`${phase}: ${name}`, async () => { | ||
| const { actions } = testBed; | ||
| await actions[phase as 'warm' | 'cold' | 'delete' | 'frozen'].enable(true); | ||
| await actions.togglePhase(phase as 'warm' | 'cold' | 'delete' | 'frozen'); |
There was a problem hiding this comment.
I know this was already here from before, but feels it should be defined just once somewhere else like in common/types/index.ts and then the rest of the places would just use that definition. I found a few other instances throughout the app that define this type over and over. Totally ignore this for now as this PR is already kind of beefy, but feels we should patch it up at some point! I can create an issue for this if you think it makes sense 🐦
There was a problem hiding this comment.
Thanks a lot for the review, @sabarasaba! That is a great catch, I'll open a separate PR for this. I started looking into this and noticed that we have some aria labels missing for frozen phase and it would have totally been caught with the correct interface.
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
…101514) * Refactored edit policy actions into separate files * Fixed types errors Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
yuliacech
added
Feature:ILM
Team:Kibana Management
|
Pinging @elastic/kibana-stack-management (Team:Stack Management) |
5 participants
Summary
Partially adresses 98031
This PR splits a previously large file
edit_policy.helpers.tsxinto separate files in theactionsfolder. The goal is to make test helpers more maintainable, readable and discoverable.List of changes:
toggleEuiSwitchmethoddata-test-subjs for consistencyedit_policy.helpers.tsxactionsobject inedit_policy.helpers.tsx:togglePhaseis now a top level functionerrorsobjectrolloverobjectsetWaitForSnapshotPolicytodeletephaseThe next step for this refactoring effort would be creating individual setup helpers for test 'areas', for example: a setup function to test form validation would not need to initialize
timelinehelpers. Similar to what @cjcenizal did in #96111.