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

[Security Solution] Make Cypress tests for rules driven by code instead of data #153692

Open
Tracked by #161505
banderror opened this issue Mar 24, 2023 · 4 comments
Open
Tracked by #161505
Labels
performance refactoring Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. technical debt Improvement of the software architecture and operational architecture test_ui_functional test

Comments

@banderror
Copy link
Contributor

Epic: #153633
Related to: (comment, comment, comment)

Summary

Make Cypress tests for rules driven by code instead of test data. Get rid of optional properties in data. This is supposed to reduce flakiness, increase speed, and improve the maintainability of our Cypress tests.

Our e2e tests are driven by test data. Whatever is or is not in the data (e.g. an instance of CustomRule) will determine what actions will be taken in the UI. Example:

      visit(RULE_CREATION);
      fillDefineCustomRuleAndContinue(this.rule);
      fillAboutRuleAndContinue(this.rule);
      fillScheduleRuleAndContinue(this.rule);
export const fillDefineCustomRuleAndContinue = (rule: CustomRule | OverrideRule) => {
  if (rule.dataSource.type === 'dataView') {
    cy.get(DATA_VIEW_OPTION).click();
    cy.get(DATA_VIEW_COMBO_BOX).type(`${rule.dataSource.dataView}{enter}`);
  }
  fillCustomQuery(rule);
  cy.get(DEFINE_CONTINUE_BUTTON).should('exist').click({ force: true });
  cy.get(CUSTOM_QUERY_INPUT).should('not.exist');
};

export const fillScheduleRuleAndContinue = (rule: CustomRule | MachineLearningRule) => {
  if (rule.runsEvery) {
    cy.get(RUNS_EVERY_INTERVAL).type('{selectall}').type(rule.runsEvery.interval);
    cy.get(RUNS_EVERY_TIME_TYPE).select(rule.runsEvery.timeType);
  }
  if (rule.lookBack) {
    cy.get(LOOK_BACK_INTERVAL).type('{selectAll}').type(rule.lookBack.interval);
    cy.get(LOOK_BACK_TIME_TYPE).select(rule.lookBack.timeType);
  }
  cy.get(SCHEDULE_CONTINUE_BUTTON).click({ force: true });
};

This approach has a few downsides:

  • We have conditional logic in functions like fillScheduleRuleAndContinue. This is commonly considered a bad practice in any automated tests because it increases complexity and non-determinism.
  • It's a bit rigid: you can't customize the behavior for a particular test, because it is common to all tests and driven by data.
  • The resulting actions that a test executes in the UI may not necessarily match the intention/expectation of the developer who writes this test. Or they may be redundant thus creating more load and increasing the chance of a flake.

The suggestion is to make tests driven by code instead of data. By using single-purpose functions (Cypress tasks, commands, etc) and combining them into higher-order functions, tests will be implementing the exact logic each of them needs.

With that in place, test data can become just normal mock data where you normally have default values for properties you don't need.

Going back to the optional properties like customQuery and runsEvery - they all will be required and have some default values. Each test will decide:

  • whether to use the default value and do something with it
  • or override the default value and do something with it
  • or just ignore it and do nothing
@banderror banderror added test performance refactoring test_ui_functional technical debt Improvement of the software architecture and operational architecture Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team 8.8 candidate labels Mar 24, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@MadameSheema
Copy link
Member

We started that effort already, but we didn't have enough hands: #143383

It would be nice to use the above PR as which things we want to maintain, which things can be dropped/changed, and also arrive at an agreement regarding the next steps section, since we are going to find similar issues.

@banderror
Copy link
Contributor Author

@MadameSheema We'll sync up on this later 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance refactoring Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. technical debt Improvement of the software architecture and operational architecture test_ui_functional test
Projects
None yet
Development

No branches or pull requests

3 participants