Re-enable and fix APM E2E tests #114831

smith · 2021-10-13T15:06:37Z

Re-enable previously disabled APM E2E tests.
Round to the nearest second in getComparisonTypes to avoid cases where a millisecond difference can change which results get shown.
Simplify error count alert tests to test the "happy path" ([APM] E2E tests for rules & alerts #79284 exists in order to expand to more tests for rule editing and creation.)
Wait for alert list API request to complete before clicking "Create rule" button when running the test to create a rule from the Stack Management UI.

I ran the e2e tests 100 times locally with no failures so I'm confident the flakiness has been addressed.

* Run previously disabled APM E2E tests on all PRs (we were previously only running them when APM files had changes.) * Remove `precise: true` from `getComparisonTypes` call which caused intermittent failures in date comparison tests. * Simplify error count alert tests to test the "happy path" (elastic#79284 exists in order to expand to more tests for rule editing and creation) * Wait for alert list API request to complete before clicking "Create rule" button when running the test to create a rule from the Stack Management UI. I ran the e2e tests 100 times locally with no failures so I'm confident the flakiness has been addressed. Fixes elastic#114419. Fixes elastic#109205.

elasticmachine · 2021-10-13T15:06:42Z

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

vars/tasks.groovy

jbudz

Do we want the buildkite version updated too?

.buildkite/scripts/pipelines/pull_request/pipeline.js

cauemarcondes · 2021-10-13T15:24:05Z

x-pack/plugins/apm/ftr_e2e/cypress/integration/power_user/rules/error_count.spec.ts

@@ -20,66 +20,55 @@ describe('Rules', () => {
      describe('when created from Service Inventory', () => {
        before(() => {
          cy.loginAsPowerUser();
+          cy.deleteAllRules();


Do you really need to call this on before and after?

It works if you don't do this, but it's better to start with a consistent state (that is, no rules present) and clean up afterwards. It's possible a previous test errors and fails to clean up after itself to this ensures that does not impact the results of this test.

This is also in line with "Cypress Best Practices": https://docs.cypress.io/guides/references/best-practices#Using-after-or-afterEach-hooks

cauemarcondes

Test changes look good to me.

smith · 2021-10-13T15:35:59Z

@jbudz

Do we want the buildkite version updated too?

.buildkite/scripts/pipelines/pull_request/pipeline.js

Updated in a988be0.

.buildkite/scripts/pipelines/pull_request/pipeline.js

smith · 2021-10-13T17:32:52Z

@elasticmachine merge upstream

sorenlouv · 2021-10-13T22:13:24Z

x-pack/plugins/apm/ftr_e2e/cypress/support/commands.ts

@@ -7,6 +7,22 @@
 import 'cypress-real-events/support';
 import { Interception } from 'cypress/types/net-stubbing';

+Cypress.Commands.add('deleteAllRules', () => {


What about namespacing commands like:

Suggested change

Cypress.Commands.add('deleteAllRules', () => {

Cypress.Commands.add('alerting:deleteAllRules', () => {

I think that will come handy as we add more commands in the future.

Oh, I suppose : is not supported. But maybe we can find another separator

I made this a command because it looked like the kind of thing that was expected in Cypress, but after reading https://docs.cypress.io/api/cypress-api/custom-commands#Best-Practices, I might just go back to making it a function in the rules test, since that's the only place it's used.

smith · 2021-10-15T14:28:17Z

.buildkite/scripts/pipelines/pull_request/pipeline.js

        /^x-pack\/plugins\/security_solution/,
        /^x-pack\/test\/security_solution_cypress/,
        /^x-pack\/plugins\/triggers_actions_ui\/public\/application\/sections\/action_connector_form/,
        /^x-pack\/plugins\/triggers_actions_ui\/public\/application\/context\/actions_connectors_context\.tsx/,
-      ]) || process.env.GITHUB_PR_LABELS.includes('ci:all-cypress-suites')


I think this change is from Prettier.

kibanamachine · 2021-10-15T16:35:00Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	2.7MB	2.7MB	+36.0B

History

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

cauemarcondes

@smith it looks like when this week is selected both options show up, while it should only show week before.

cauemarcondes

LGTM, but we should fix the comparison bug on another PR.

smith · 2021-10-15T18:25:21Z

LGTM, but we should fix the comparison bug on another PR.

Thanks @cauemarcondes. I opened #115247.

* Re-enable previously disabled APM E2E tests. * Round to the nearest second in `getComparisonTypes` to avoid cases where a millisecond difference can change which results get shown. * Simplify error count alert tests to test the "happy path" (elastic#79284 exists in order to expand to more tests for rule editing and creation.) * Wait for alert list API request to complete before clicking "Create rule" button when running the test to create a rule from the Stack Management UI. I ran the e2e tests 100 times locally with no failures so I'm confident the flakiness has been addressed. Fixes elastic#114419. Fixes elastic#109205.

kibanamachine · 2021-10-15T18:30:04Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

* Re-enable previously disabled APM E2E tests. * Round to the nearest second in `getComparisonTypes` to avoid cases where a millisecond difference can change which results get shown. * Simplify error count alert tests to test the "happy path" (#79284 exists in order to expand to more tests for rule editing and creation.) * Wait for alert list API request to complete before clicking "Create rule" button when running the test to create a rule from the Stack Management UI. I ran the e2e tests 100 times locally with no failures so I'm confident the flakiness has been addressed. Fixes #114419. Fixes #109205. Co-authored-by: Nathan L Smith <[email protected]>

smith added 2 commits October 13, 2021 09:57

remove duplicate contains call

d82d298

smith added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 labels Oct 13, 2021

smith requested a review from a team as a code owner October 13, 2021 15:06

botelastic bot added the Team:APM All issues that need APM UI Team support label Oct 13, 2021

commit tasks change

1041281

smith requested a review from a team as a code owner October 13, 2021 15:11

spalger suggested changes Oct 13, 2021

View reviewed changes

vars/tasks.groovy Outdated Show resolved Hide resolved

jbudz reviewed Oct 13, 2021

View reviewed changes

cauemarcondes reviewed Oct 13, 2021

View reviewed changes

cauemarcondes approved these changes Oct 13, 2021

View reviewed changes

only run when APM changes

077c09a

smith requested a review from spalger October 13, 2021 15:34

re-enable in buildkite as well

a988be0

spalger approved these changes Oct 13, 2021

View reviewed changes

jbudz approved these changes Oct 13, 2021

View reviewed changes

smith mentioned this pull request Oct 13, 2021

[APM] Run APM e2e test uncondtionally #114419

Closed

sorenlouv reviewed Oct 13, 2021

View reviewed changes

.buildkite/scripts/pipelines/pull_request/pipeline.js Outdated Show resolved Hide resolved

fix types

e767b9f

Merge branch 'master' into nls/fix-e2e

3bd7e19

tylersmalley mentioned this pull request Oct 13, 2021

[renovate] Configure Cypress #114880

Merged

sorenlouv reviewed Oct 13, 2021

View reviewed changes

sorenlouv approved these changes Oct 13, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master' into nls/fix-e2e

e8af046

smith added 4 commits October 14, 2021 13:57

clean up helpers

c4b5630

Merge remote-tracking branch 'upstream/master' into nls/fix-e2e

c498c94

round

44f8d6f

Merge remote-tracking branch 'upstream/master' into nls/fix-e2e

78e91fa

smith requested a review from cauemarcondes October 15, 2021 14:27

smith commented Oct 15, 2021

View reviewed changes

cauemarcondes requested changes Oct 15, 2021

View reviewed changes

cauemarcondes approved these changes Oct 15, 2021

View reviewed changes

smith merged commit 07777b9 into elastic:master Oct 15, 2021

smith deleted the nls/fix-e2e branch October 15, 2021 18:25

kibanamachine mentioned this pull request Oct 15, 2021

[7.x] Re-enable and fix APM E2E tests (#114831) #115251

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-enable and fix APM E2E tests #114831

Re-enable and fix APM E2E tests #114831

smith commented Oct 13, 2021 •

edited

Loading

elasticmachine commented Oct 13, 2021

jbudz left a comment

cauemarcondes Oct 13, 2021

smith Oct 13, 2021

sorenlouv Oct 13, 2021

cauemarcondes left a comment

smith commented Oct 13, 2021

smith commented Oct 13, 2021

sorenlouv Oct 13, 2021

sorenlouv Oct 13, 2021

smith Oct 14, 2021

smith Oct 15, 2021

kibanamachine commented Oct 15, 2021

cauemarcondes left a comment

cauemarcondes left a comment

smith commented Oct 15, 2021

kibanamachine commented Oct 15, 2021

	Cypress.Commands.add('deleteAllRules', () => {
	Cypress.Commands.add('alerting:deleteAllRules', () => {

Re-enable and fix APM E2E tests #114831

Re-enable and fix APM E2E tests #114831

Conversation

smith commented Oct 13, 2021 • edited Loading

elasticmachine commented Oct 13, 2021

jbudz left a comment

Choose a reason for hiding this comment

cauemarcondes Oct 13, 2021

Choose a reason for hiding this comment

smith Oct 13, 2021

Choose a reason for hiding this comment

sorenlouv Oct 13, 2021

Choose a reason for hiding this comment

cauemarcondes left a comment

Choose a reason for hiding this comment

smith commented Oct 13, 2021

smith commented Oct 13, 2021

sorenlouv Oct 13, 2021

Choose a reason for hiding this comment

sorenlouv Oct 13, 2021

Choose a reason for hiding this comment

smith Oct 14, 2021

Choose a reason for hiding this comment

smith Oct 15, 2021

Choose a reason for hiding this comment

kibanamachine commented Oct 15, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

cauemarcondes left a comment

Choose a reason for hiding this comment

cauemarcondes left a comment

Choose a reason for hiding this comment

smith commented Oct 15, 2021

kibanamachine commented Oct 15, 2021

💚 Backport successful

smith commented Oct 13, 2021 •

edited

Loading