[APM UI] Rephrase BDD steps for RUM scenarios

[mdelapenya]

Summary

This PR simplifies three scenarios, merging them into just one, which will reduce the build time because we will be running just one scenario instead of three doing exactly the same: we will run the tests and check for the three components in different steps of the same scenario.

Besides that, we are rephrasing the existing scenarios, willing to open a discussion about refining them.

[mdelapenya]

Writing down my thoughts here, because I cannot add them to the proper lines in the diff.

I noticed that the method:

Then(`should have correct client metrics`, () => {
  const clientMetrics = '[data-cy=client-metrics] .euiStat__title';

  // wait for all loading to finish
  cy.get('kbnLoadingIndicator').should('not.be.visible');
  cy.get('.euiStat__title', { timeout: DEFAULT_TIMEOUT }).should('be.visible');
  cy.get('.euiSelect-isLoading').should('not.be.visible');
  cy.get('.euiStat__title-isLoading').should('not.be.visible');

  cy.get(clientMetrics).eq(2).should('have.text', '55 ');

  cy.get(clientMetrics).eq(1).should('have.text', '0.08 sec');

  cy.get(clientMetrics).eq(0).should('have.text', '0.01 sec');
});

which relates to the BDD step:

    Then should have correct client metrics

is almost the same as:

Then(`it displays relevant client metrics`, () => {
  const clientMetrics = '[data-cy=client-metrics] .euiStat__title';

  // wait for all loading to finish
  cy.get('kbnLoadingIndicator').should('not.be.visible');
  cy.get('.euiStat__title-isLoading').should('not.be.visible');

  cy.get(clientMetrics).eq(2).should('have.text', '7 ');

  cy.get(clientMetrics).eq(1).should('have.text', '0.07 sec');

  cy.get(clientMetrics).eq(0).should('have.text', '0.01 sec');
});

which relates to the BDD step:

    Then it displays relevant client metrics

I wonder if both methods and scenarios could be refactored into one, or in a helper method with parameters.

[mdelapenya]

Another question that I have:

  cy.get(clientMetrics).eq(2).should('have.text', '7 ');

I noticed there is a whitespace after the value. Is this on purpose and needed?

[mdelapenya]

Hey @shahzad31, any feedback here? I'd like to make progress with these scenarios

Thanks!

[shahzad31]

Another question that I have:

  cy.get(clientMetrics).eq(2).should('have.text', '7 ');

I noticed there is a whitespace after the value. Is this on purpose and needed?

i think space was part of text fetch from component, maybe we can avoid this by using trim string method.

[shahzad31]

Writing down my thoughts here, because I cannot add them to the proper lines in the diff.

I noticed that the method:

Then(`should have correct client metrics`, () => {
  const clientMetrics = '[data-cy=client-metrics] .euiStat__title';

  // wait for all loading to finish
  cy.get('kbnLoadingIndicator').should('not.be.visible');
  cy.get('.euiStat__title', { timeout: DEFAULT_TIMEOUT }).should('be.visible');
  cy.get('.euiSelect-isLoading').should('not.be.visible');
  cy.get('.euiStat__title-isLoading').should('not.be.visible');

  cy.get(clientMetrics).eq(2).should('have.text', '55 ');

  cy.get(clientMetrics).eq(1).should('have.text', '0.08 sec');

  cy.get(clientMetrics).eq(0).should('have.text', '0.01 sec');
});

which relates to the BDD step:

    Then should have correct client metrics

is almost the same as:

Then(`it displays relevant client metrics`, () => {
  const clientMetrics = '[data-cy=client-metrics] .euiStat__title';

  // wait for all loading to finish
  cy.get('kbnLoadingIndicator').should('not.be.visible');
  cy.get('.euiStat__title-isLoading').should('not.be.visible');

  cy.get(clientMetrics).eq(2).should('have.text', '7 ');

  cy.get(clientMetrics).eq(1).should('have.text', '0.07 sec');

  cy.get(clientMetrics).eq(0).should('have.text', '0.01 sec');
});

which relates to the BDD step:

    Then it displays relevant client metrics

I wonder if both methods and scenarios could be refactored into one, or in a helper method with parameters.

@mdelapenya i am fine with merging both of these methods.

[mdelapenya]

Another question that I have:

  cy.get(clientMetrics).eq(2).should('have.text', '7 ');

I noticed there is a whitespace after the value. Is this on purpose and needed?

i think space was part of text fetch from component, maybe we can avoid this by using trim string method.

You mean at the plugin code? or in the tests? I'd prefer the 1st one

[mdelapenya]

Hey @shahzad31 I'd love a pair of 👀 reviewing the typescript code after refactor: I had to skip presubmit checks when creating the commit because not sure how to fix them.

[elasticmachine]

Pinging @elastic/uptime (Team:uptime)

[elasticmachine]

Pinging @elastic/apm-ui (Team:apm)

[smith]

This looks fine to me as long as everything is still relevant and passing.

[mdelapenya]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: c95b651

Build metrics

distributable file count

id	value	diff	baseline
total	45456	+1	45455

History

• 💚 Build #65341 succeeded 1de2f06
• 💚 Build #65329 succeeded 21be449
• 💚 Build #63064 succeeded 7cd1d1b

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

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 72492 or prevent reminders by adding the backport:skip label.

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 72492 or prevent reminders by adding the backport:skip label.