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.

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?

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

Thanks!

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.

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.

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

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.

Pinging @elastic/uptime (Team:uptime)

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

@elasticmachine merge upstream

💚 Build Succeeded

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

Build metrics

distributable file count

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

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.

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.