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] Minimal e2e setup with Cypress #43463

Open
wants to merge 13 commits into
base: master
from

Conversation

@sqren
Copy link
Member

commented Aug 16, 2019

This is a super minimal setup to get us started with Cypress. I initially wanted to automate everything up front (data creation, data ingestion, running on CI etc) but I think it's okay to start out with something super minimal that is easier to modify.

I recommend you start reading the readme in apm/cypress/README.md and then try to run cypress.

If you are interested to learn how the data that is ingested via replay.js is generated you can have a look here: elastic/apm-integration-testing#565

TODO

  • Avoid Kibana restarting when changing tests
  • Script for ingesting previously generated data
  • Cloud instance with pre-recorded data
  • Instructions for connecting to cloud instance
  • Instructions for generating new data and ingesting it with script
  • Typescript support in Cypress
  • Fix eslint and typescript errors
  • Store values as snapshots instead hardcoded values (much easier to bulk update when data set changes)
  • Make it run on CI
  • Updated timestamps in static data Hardcode specific time range
  • Add some actual tests Dummy tests added.

@sqren sqren requested a review from elastic/apm-ui as a code owner Aug 16, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2019

@sqren sqren force-pushed the sqren:apm-e2e branch from 1b58c91 to 71a30eb Aug 17, 2019

@dgieselaar

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2019

Really excited about us having the opportunity to write less painful E2E tests 💪🏻Two things:

@sqren sqren force-pushed the sqren:apm-e2e branch from 71a30eb to da19820 Aug 17, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2019

@sqren

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2019

SIEM added a package.json to their plugin folder for their cypress tests, maybe sensible for us as well: https://github.com/elastic/kibana/tree/master/x-pack/legacy/plugins/siem/package.json

I already added a package.json file (was semi-required by cypress) but I chose not to add dependencies since it would require us to install them as a separate step form yarn kbn bootstrap.

@sqren

This comment has been minimized.

Copy link
Member Author

commented Aug 20, 2019

Cypress has an API that allows you to set the current time

Nice find! That's much simpler than transforming the input data

@sqren sqren force-pushed the sqren:apm-e2e branch from da19820 to 836a325 Sep 2, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 2, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2019

@sqren sqren force-pushed the sqren:apm-e2e branch from dda4c71 to 087a575 Sep 4, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

@dgieselaar

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

To get back to my earlier comment, it would be great if we could have some pre-recorded data hosted somewhere that this script can download. It would make it a little faster to get started. Or maybe just ES snapshots? Or is that out of scope for this version?

@sqren

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

To get back to my earlier comment, it would be great if we could have some pre-recorded data hosted somewhere that this script can download.

@dgieselaar Did you see this file x-pack/legacy/plugins/apm/cypress/ingest-data/captured-events-small.json.zip? Or did you mean something else?

@@ -181,6 +181,7 @@ export default class ClusterManager {
ignored: [
/[\\\/](\..*|node_modules|bower_components|public|__[a-z0-9_]+__|coverage)[\\\/]/,
/\.test\.js$/,
/\/cypress\//,

This comment has been minimized.

Copy link
@sqren

sqren Sep 4, 2019

Author Member

This keeps Kibana from starting whenever files in cypress are modified. Probably needs a tighter check but works for now

@dgieselaar

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

@sqren no, missed it! my bad. file was collapsed because it's binary. nice! are we okay with committing zip files to the repo?

@sqren

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

are we okay with committing zip files to the repo?

It decreased the file size from 915kb to 83kb - so that seems like a pretty big saving. OTOH git probably does compression on its own, so might not make a difference after all.
What are your concern with a zip vs json? My thinking is that this will be automated eventually, so we won't have to manually unzip it either way.

@dgieselaar

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

@sqren

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

@dgieselaar You are totally right though. The pre-recorded data I've added here is just a small fraction of the original file. Initially I wanted to use the big file (that spans several hours) but it is almost 100mb. I'm not sure more data is better necessarily. And adding it to kibana repo will cause slowness all over the place (imagine the CI's that clones the repo 100s of times a day - often from clean slate).

I don't know what the best solution is: upload the big data file somewhere else, or just go with the smaller file?

@dgieselaar

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

@sqren sqren force-pushed the sqren:apm-e2e branch from d2ce674 to 00e4388 Sep 6, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2019

@sqren sqren force-pushed the sqren:apm-e2e branch from eeeeb39 to 6245b53 Sep 10, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 15, 2019

@sqren sqren force-pushed the sqren:apm-e2e branch from 5a828c6 to 1ee1228 Sep 16, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

@@ -0,0 +1,21 @@
{
"name": "apm-cypress",

This comment has been minimized.

Copy link
@spalger

spalger Sep 17, 2019

Member

This package doesn't get its dependencies installed by bootstrap, and having dependencies in a location like this isn't how we've done this in the past, but I'm intrigued by the idea...

This comment has been minimized.

Copy link
@sqren

sqren Sep 17, 2019

Author Member

Yeah, I didn't want to bother the rest of kibana with the dependencies our e2e project needed. And I was running into a whole host of typescript issues. This way it's a completely separate project that just happens to live inside kibana. It's has the benefit of being very portable (we are not 100% sure whether it should live in kibana or somewhere else since it acts as integration test between several components)

@spalger
Copy link
Member

left a comment

TS Project changes LGTM

I think we might want to talk about where we put cypress tests so that they can share a cypress install with other plugins, and we might want to move them away from the source (maybe into a package or the test directory) so that they don't try to import code from the plugin which will break things pretty quickly and really slow things down.

@sqren

This comment has been minimized.

Copy link
Member Author

commented Sep 18, 2019

@elasticmachine merge upstream

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2019

@sqren

This comment has been minimized.

Copy link
Member Author

commented Sep 18, 2019

@elasticmachine merge upstream

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2019

@dgieselaar
Copy link
Contributor

left a comment

Gotta say, Cypress is really nice. I just have some nits and questions, mostly.

x-pack/legacy/plugins/apm/cypress/README.md Outdated Show resolved Hide resolved
x-pack/legacy/plugins/apm/readme.md Outdated Show resolved Hide resolved
x-pack/legacy/plugins/apm/cypress/support/commands.js Outdated Show resolved Hide resolved
x-pack/legacy/plugins/apm/cypress/snapshots.js Outdated Show resolved Hide resolved
x-pack/legacy/plugins/apm/cypress/support/commands.js Outdated Show resolved Hide resolved
sqren added 8 commits Sep 14, 2019

@sqren sqren force-pushed the sqren:apm-e2e branch from 00660ce to bde96c1 Sep 19, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2019

sqren added 2 commits Sep 20, 2019
@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2019

sqren added 2 commits Sep 20, 2019
@sqren

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2019

@dgieselaar All comments addressed.

@sqren sqren requested a review from dgieselaar Sep 20, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2019

@elasticmachine

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.