[ILM] Refactor edit_policy client integration tests into separate feature files

[yuliacech]

Addresses #88593
Continues work started in PR#91657

Summary

This PR refactors the 2nd large edit_policy.test.ts file into separate files categorized by features of the form.

Folder structure before

edit_policy
├── constants.ts
├── edit_policy.helpers.tsx
├── edit_policy.test.ts
├── form_validation
│   ├── cold_phase_validation.test.ts
│   ├── delete_phase_validation.ts
│   ├── hot_phase_validation.test.ts
│   ├── policy_name_validation.test.ts
│   └── warm_phase_validation.test.ts
└── reactive_form
    ├── node_allocation.test.ts
    └── reactive_form.test.ts

Folder structure after

edit_policy
├── constants.ts
├── edit_policy.helpers.tsx
├── features
│   ├── cold_phase.test.ts
│   ├── delete_phase.test.ts
│   ├── json_flyout.test.ts
│   ├── node_allocation.test.ts
│   ├── rollover.test.ts
│   ├── searchable_snapshots.test.ts
│   ├── timeline.test.ts
│   └── warm_phase.test.ts
├── form_validation
│   ├── cold_phase_validation.test.ts
│   ├── delete_phase_validation.ts
│   ├── error_indicators.test.ts
│   ├── hot_phase_validation.test.ts
│   ├── policy_name_validation.test.ts
│   └── warm_phase_validation.test.ts
└── serialization
    └── policy_serialization.test.ts

Description of changes

• Added error indicators tests to form_validation folder
• Split reactive_form.test.ts into smaller files and renamed reactive_form folder to features that now contains:
  • warm phase tests
  • cold phase tests
  • delete phase tests
  • node allocation tests
  • request flyout tests
  • rollover tests
  • searchable snapshots tests
  • timeline tests
• Added serialization folder with serialization tests

Future work

• Another improvement should be done for helpers file that is has now already reached ~400 lines. Now that we have separated tests into features, some helper methods can be moved to a corresponding file where they are used leaving the general helpers file small and readable.
• Many small files have a downside of repeating boilerplate of setting up the testbed. Test bed setup function could be more reusable with some defaults to avoid code duplication.

[elasticmachine]

Pinging @elastic/es-ui (Team:Elasticsearch UI)

[cjcenizal]

@yuliacech Could you collaborate with @cuff-links and get his input on the readability and scannability of these tests? In our retro we identified a need to better coordinate our automated test coverage with our manual test coverage. I think one step in this direction will be to make it easier for QA engineers to understand the UX flows being tested by our automated tests, so they can compare them against the UX flows covered by TestRail. I think this will be a long-term process so no need to get it right in this particular PR, but I think this is a good opportunity to start gathering this feedback. WDYT?

[yuliacech]

Thanks for pointing this out, @cjcenizal! I'll make sure to get @cuff-links input and feedback for this.

[jloleysens]

This is looking really great @yuliacech ! 🎉 The new structure is really cool.

I agree with CJ's comment regarding readability of tests, to my mind the thing that could use more careful thinking through is how the describe and test names serialize into something really clear for readers of the test results. At least, I know I could do better here! I think that consistency is of primary importance but I'd be interested to hear from @cuff-links (and you) too! Not sure we need to block this PR on that though - we can always do another pass.

Would you mind converting the PR description to use ASCII-style tree (generated with CLI tree) for the new test dir structure:

client_integration
├── app
│   ├── app.helpers.tsx
│   └── app.test.ts
├── edit_policy
│   ├── constants.ts
│   ├── edit_policy.helpers.tsx
│   ├── edit_policy.test.ts
│   ├── form_validation
│   │   ├── cold_phase_validation.test.ts
│   │   ├── delete_phase_validation.ts
│   │   ├── hot_phase_validation.test.ts
│   │   ├── policy_name_validation.test.ts
│   │   └── warm_phase_validation.test.ts
│   └── reactive_form
│       ├── node_allocation.test.ts
│       └── reactive_form.test.ts
└── helpers
    ├── http_requests.ts
    ├── index.ts
    └── setup_environment.ts

5 directories, 15 files

This would really help me to grok the structural changes introduced in this PR!

The main thing I'd like to get your thoughts on for my review, before approving (because mostly this looks pretty great to me!!) is the the reactive_form.test.ts file. At the moment my concern is that it is not 100% clear when I should add something here vs another feature test file. This can be addressed by a comment or README.md to capture the constraints of when to add something there. Alternatively, the tests in reactive_form.test.ts could be moved to separate or existing feature test files. For instance; min age tooltip could be moved to rollover file, JSON flyout could be its own feature and the special delete phase behaviour could also be its own file. Let me know what you think of one of those two directions!

[yuliacech]

@elasticmachine merge upstream

[kibanamachine]

merge conflict between base and head

[yuliacech]

Thanks a lot for your review, @jloleysens! I agree about reactive_form file, I thought that the name is not very clear. So I split it into smaller files as you suggested. Also, great idea for the folder structure representation, I've haven't used tree before and it's really useful.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: e014739
• Storybooks Preview
• Documentation Changes

Metrics [docs]

✅ unchanged

History

• 💚 Build #109995 succeeded 934ce2d
• 💚 Build #109940 succeeded f75fc23
• 💚 Build #109453 succeeded 16e4e50
• 💚 Build #109421 succeeded 1c324d0

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[jloleysens]

Thanks for making those changes @yuliacech ! Happy for this to be merged!

[yuliacech]

I've merged this PR for now and will plan a feedback session with @cuff-links when he's back.