elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.99k stars 8.24k forks source link

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

Open banderror opened 1 year ago

banderror commented 1 year ago

Epic: https://github.com/elastic/kibana/issues/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:

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:

elasticmachine commented 1 year ago

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

elasticmachine commented 1 year ago

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

MadameSheema commented 1 year ago

We started that effort already, but we didn't have enough hands: https://github.com/elastic/kibana/pull/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 commented 1 year ago

@MadameSheema We'll sync up on this later 👍