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] Adds Jest tests for the rules editor flyout components #21636

Merged
merged 1 commit into from
Aug 6, 2018

Conversation

peteharverson
Copy link
Contributor

Adds Jest tests for the components on the rule editor flyout, as used in the Anomalies table, created in #20339.

Tests added for components:

  • actions section
  • condition expression
  • conditions section
  • scope explression
  • scope section
  • detector description list
  • delete rule modal
  • rule action panel
  • rule editor flyout

Addresses the first item in #20831

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

jest.mock('../../services/job_service', () => ({
mlJobService: {
getJob: () => {
return {
Copy link
Contributor

Choose a reason for hiding this comment

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

total nit but this return object could be a separate JSON file which we import. resonates with the idea James' mentioned that something like a mock for mlJobService could be made reusable for other tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, as some of these mocked functions start needing to be reused I agree it would be a good idea to move things like test job configs into separate files. We'd need a wide range of job configs to cover all the things we're testing - here for example I want a detector with no partitioning fields and a single rule.

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM, just added a nitpick comment about possibly putting mock objects in separate json files.

@peteharverson peteharverson force-pushed the ml-rules-editor-tests branch from daf7e8d to d94f24c Compare August 6, 2018 08:34
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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.

4 participants