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

Move from cmd/e2e runner to native Ginkgo runner #5670

Closed
zmerlynn opened this issue Mar 19, 2015 · 10 comments · Fixed by #7462
Closed

Move from cmd/e2e runner to native Ginkgo runner #5670

zmerlynn opened this issue Mar 19, 2015 · 10 comments · Fixed by #7462
Assignees
Labels
area/test-infra priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@zmerlynn
Copy link
Member

This is an off-shoot of onsi/ginkgo#146 (comment). I actually think that the structure of the existing cmd/e2e runner is wrong, and that we need to work towards getting the native Ginkgo runner working with our suite(s). This has the following advantages:

  • Right now, we pretty much just generate one suite. We really want to target at least a second group, a stress group. We could do this hackily with --ginkgo.focus, but we should have a better tool.
  • We've extended our own runner a couple of times just to add things the base runner already has. We should stop doing that.
  • We're eventually going to want things the normal runner already has, like parallelism.

I'm not as well versed in the blockers. I remember part of it is as simple as the command-line flags: we take a host of flags to the tests and didn't want to take them as environment variables, but I think this one was easily solved? But I think the other blockers were just someone taking the time to actually do the work.

cc @onsi @rrati @roberthbailey @filbranden @fabioy

@zmerlynn zmerlynn added priority/backlog Higher priority than priority/awaiting-more-evidence. area/test-infra labels Mar 19, 2015
@satnam6502
Copy link
Contributor

@ixdy

@filbranden
Copy link
Contributor

Regarding the flags, Ginkgo supports custom flags. See the third example about custom arguments and/or flags with <PASS-THROUGH> here:
http://onsi.github.io/ginkgo/#running-tests

I think most of what we need to do here is move them from cmd/e2e to the actual tests in test/e2e, maybe in a common package that is sourced by all the tests.

@onsi
Copy link
Contributor

onsi commented Mar 19, 2015

yep - Ginkgo will let you specify custom flags.

ginkgo is basically orchestrating and running go test under the hood. I'd recommend structuring the tests as a go tests in _test files though that isn't strictly necessary.

If you have two very different populations of tests (e.g. integration tests vs stress tests) make them different suites and pull out any helpers etc. into a shared library. Ginkgo makes running multiple suites very easy and you won't have to mess with --focus or --skip.

Also: try to use BeforeSuite and AfterSuite to manage setup and clean up. This has many advantageous properties including (for example) guaranteeing that AfterSuite runs on interrupt. I have an integration suite that runs against a live cluster that carefully cleans up its mess when I ^C.

I can help when y'all get to parallelism. It's very easy to point a parallel suite at a singleton external resource (example a cluster) and then shard the tests using GinkgoParallelNode. I use this to namespace the load on the cluster I'm testing by parallel-node. This helps the parallel nodes avoid stepping on each other's toes. And, again, ^C just works - each parallel-node will tear down its namespaced subset of data.

@onsi
Copy link
Contributor

onsi commented Mar 19, 2015

Also - I'd be happy to add additional features to support the stress test scenario. ginkgo stress sounds like a perfect fit and would be happy to explore that with y'all.

@ghost
Copy link

ghost commented Apr 14, 2015

cc: quinton-hoole

@jayunit100
Copy link
Member

Related to #6552 and this issue --- on my box, looks like running the cmd/e2e... tests from source now breaks due to gopath docker/libcontainer library errors. not sure why. but I think it underscores the importance of addressing this issue. Thanks for looking into it !

@ghost
Copy link

ghost commented Apr 23, 2015

@ixdy how is this progressing?

@ixdy
Copy link
Member

ixdy commented May 1, 2015

#7462 was reverted because it broke Jenkins; it turns out that using the native Ginkgo runner takes a bit more work than originally expected:

  1. The ginkgo binary needs to be built as part of the release (done in Build github.com/onsi/ginkgo/ginkgo as a part of the release #7593).
  2. The e2e tests need to be built as part of the release, using ginkgo build.
  3. Finally we can update hack/ginkgo-e2e.sh to use the ginkgo binary (from the first step) and the built tests (from the second step). Optionally this could use the unbuilt tests (building them first), if the source is available.

Step two is going to take a bit of careful thought, since it needs to occur after ginkgo is built, and will ideally share a lot of the existing shell code for building the rest of the binaries so that it remains portable.

@zmerlynn
Copy link
Member Author

@ixdy: Any progress on this?

@ixdy
Copy link
Member

ixdy commented Jun 15, 2015

Still more cleanup to be done, but the base issue is done.

@ixdy ixdy closed this as completed Jun 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/test-infra priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants