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

[APM] Run APM e2e test uncondtionally #114419

Closed
sorenlouv opened this issue Oct 10, 2021 · 5 comments · Fixed by #114831
Closed

[APM] Run APM e2e test uncondtionally #114419

sorenlouv opened this issue Oct 10, 2021 · 5 comments · Fixed by #114831
Assignees
Labels
Team:APM All issues that need APM UI Team support

Comments

@sorenlouv
Copy link
Member

sorenlouv commented Oct 10, 2021

In #113618 we had to disable APM's e2e test because they had started to fail. The cause was an update of hapi was upgraded to 20.2.0 which included a bug.
In #114414 hapi will be upgraded again with a patch that should fix APM's e2e's.

To avoid getting in this situation again we should run the e2e's unconditionally - and not only when APM code is changed.

This can be done by removing the whenChanged check:

kibana/vars/tasks.groovy

Lines 149 to 155 in 8a80c85

whenChanged([
'x-pack/plugins/apm/',
]) {
if (githubPr.isPr()) {
task(kibanaPipeline.functionalTestProcess('xpack-APMCypress', './test/scripts/jenkins_apm_cypress.sh'))
}
}

@sorenlouv sorenlouv added [zube]: 7.16 Team:APM All issues that need APM UI Team support labels Oct 10, 2021
@elasticmachine
Copy link
Contributor

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

@smith
Copy link
Contributor

smith commented Oct 11, 2021

APM E2E was disabled again in #114544.

@smith
Copy link
Contributor

smith commented Oct 11, 2021

The latest failures don't seem to be from the issue with Hapi from before, but a failure on the error count test. Looking into this.

@smith smith self-assigned this Oct 11, 2021
@smith
Copy link
Contributor

smith commented Oct 12, 2021

This also fails intermittently: #109205. Will try to fix in the PR to fix this issue.

smith added a commit to smith/kibana that referenced this issue Oct 13, 2021
* 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.
@smith
Copy link
Contributor

smith commented Oct 13, 2021

I'm re-enabling them but not running them unconditionally. @spalger wrote a comment on the PR:

Sorry, but we don't run cypress tests on all PRs. That's the trade off of using cypress.

In a follow-up conversation in Slack:

Yeah, that's the deal that was made when cypress was chosen to begin with
Cypress is not supported, it's not recommended, it's not encouraged, and every chance we get to ask teams to use the tools we do support we take

Since running Cypress tests unconditionally is not supported, we are still open to the failure this issue was created to address, so our options are either:

  • Use the stock FTR tools (selenium/webdriver/mocha/FTR) and rewrite all of our tests
  • Continue to use Cypress and understand that if changes are made that break Cypress we won't be aware of them until somebody makes a PR that changes APM files

For now we're continuing to use Cypress and are aware of the risk. Closing this issue.

@smith smith closed this as completed Oct 13, 2021
smith added a commit that referenced this issue Oct 15, 2021
* 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.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 15, 2021
* 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 added a commit that referenced this issue Oct 15, 2021
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:APM All issues that need APM UI Team support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants