Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Functional tests - refactor structure of ML and Transform test files #75307

Merged
merged 12 commits into from
Aug 20, 2020

Conversation

pheyos
Copy link
Member

@pheyos pheyos commented Aug 18, 2020

Summary

This PR refactors the structure of ML and Transform functional UI test files.

Details

This is mainly about not having a test (via it) for actions that are rather a test step, but make the tests be a high level item that can potentially contain many test steps. That way the test files are better structured and easier to maintain. The tests inside a suite can still be dependent on each other, i.e. if the first test fails, the second one is likely to fail as well as some prerequisite isn't met.
In the past, we used many tests to get more visibility for where a test failure occurred (particularly interesting in longer workflow tests). Since there's no test step support available in the test framework but we still don't want to lose the ability to quickly track down the failure step, a new test step logging is introduced with this PR via testExecution.logTestStep (available in the ml and the transform service). For starters, this just takes over each it title and logs it as a test step. A few new high level it entries have been added.
One example for the new test structure:
image

Note: More changes are planned for the future, e.g. separating out some of the clone, edit and delete tests to their own test files.

Other changes

A few more small refactorings are also part of this PR:

  • Replace type guards with ! notation
  • Split service ml.common into ml.common_ui and ml.common_api
  • Fix the calendar selection method in AD job wizards from combobox.setCustom (which is designed to add new entries like in our job group input) to combobox.set (which should be used to pick an existing entry from the list).

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@pheyos
Copy link
Member Author

pheyos commented Aug 18, 2020

Checking test stability in flaky test runner jobs:

  • ML UI ... => no failure in 50 runs ✔️
  • TFM UI ... => no failure in 50 runs ✔️
  • ML + TFM API integration ... => no ML failure in 50 runs ✔️ (one unrelated security API test failure)

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactor LGTM. Ran a few of the suites locally too.

@qn895
Copy link
Member

qn895 commented Aug 19, 2020

LGTM 💯

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ⚡

@pheyos pheyos merged commit c88c732 into elastic:master Aug 20, 2020
@pheyos pheyos deleted the test_structure_refactoring branch August 20, 2020 08:07
pheyos added a commit to pheyos/kibana that referenced this pull request Aug 20, 2020
…iles (elastic#75307)

This PR refactors the structure of ML and Transform functional UI test files.
pheyos added a commit that referenced this pull request Aug 20, 2020
…test files (#75307) (#75529)

* [ML] Functional tests - refactor structure of ML and Transform test files (#75307)

This PR refactors the structure of ML and Transform functional UI test files.

* [ML] Functional tests - fix imports in results API tests (#75533)

This PR fixes the broken COMMON_REQUEST_HEADERS in the recently added results API tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants