⚠ Bring in testing framework #714
Conversation
We use [ginkgo](http://onsi.github.io/ginkgo/) and [gomega](http://onsi.github.io/gomega/) for testing. We generate some boilerplate with: ``` mkdir integration cd integration ginkgo bootstrap ginkgo generate integration ``` We use [gexec](http://onsi.github.io/gomega/#gexec-testing-external-processes) to compile and run the CLI under test, and to inspect its output. We use `dep ensure` to ensure that all our dependencies are properly vendored. From now on, this will be our workflow with every commit.
As a separate commit, to make review easier.
We use [ginkgo](http://onsi.github.io/ginkgo/) and [gomega](http://onsi.github.io/gomega/) for testing. We generate some boilerplate with: ``` mkdir integration cd integration ginkgo bootstrap ginkgo generate integration ``` We use [gexec](http://onsi.github.io/gomega/#gexec-testing-external-processes) to compile and run the CLI under test, and to inspect its output. We use `dep ensure` to ensure that all our dependencies are properly vendored. From now on, this will be our workflow with every commit.
As a separate commit, to make review easier.
To start an apiserver:
```
apiServer := APIServer{Path: "/path/to/my/apiserver/binary"}
session, err := apiServer.Start("tcp://whereever.is.my.etcd:port")
Expect(err).NotTo(HaveOccurred())
```
When you're done testing against that apiserver:
```
session.Terminate().Wait()
```
...or if you prefer:
```
gexec.Terminate()
```
...which will terminate not only this apiserver, but also all other
command sessions you started in this test.
This can be started and stopped the same way as the apiserver.
Create a new set of test fixtures by doing:
```
f := test.NewFixtures("/path/to/etcd", "/path/to/apiserver")
```
Before running your integration tests, start all your fixtures:
```
err := f.Start()
Expect(err).NotTo(HaveOccurred())
```
Now that you have started your etcd and apiserver, you'll find the
apiserver listening locally on the default port. When you're done with
your testing, stop and clean up:
```
err := f.Stop()
Expect(err).NotTo(HaveOccurred())
```
We're using concourse because we happen to have a concourse deployment available. You can look at it here: https://wings.concourse.ci/teams/k8s-c10s/
We're not exercising the test framework yet, but it's in place. Our democli expects its test assets to be in `./assets/bin`. We have a script `./scripts/download-binaries.sh` which will populate that directory from a google storage bucket. Once those assets are in place, you can run tests with `./scripts/run-tests.sh`.
os.tempDir() gives the path to the temporary directory, it does not create a random temporary directory.
Using tcp:// does not work.
Testing the lifecycle of our fixtures with the real binaries. Test if we can start the fixtures, the porcesses actually listen on the ports and if we tear down all the parts successfully again.
- Store stdout,stderr in private buffers - Configure the etcURL on construction instead of at start time - Handle the creation of the temporary directory (for the data directory) internally
- Store stdout,stderr in private buffers - Configure the etcURL on construction instead of at start time
Instead of the separate {Etcd,APIServer}StartStopper use the unified
interface FixtureProcess
We are now returning an error instead of using an Expectation inline.
- Start() should only return when the process is actually up and
listening
- It may take some time to tear down a process, so we increased the
timeout for Stop() (to some random number)
- We make sure Std{Out,Err} is properly initialized, we should not rely
on Ginkgo/Gomega to do that for us
... as they have been unified into the FixtureProcess interface and thus they are not needed anymore.
We use the standard go client for kubernetes `client-go`. To vendor it and all its denpendecies we use ``` dep ensure -add k8s.io/client-go@5.0.0 ``` We create a new cobra command with ``` cobra add listPods ``` Note: The new command in cmd/listPods.go uses [the "magic" function init()](https://golang.org/ref/spec#Package_initialization) to register itself.
As a separate commit, to make review easier.
Performance tests are now skipped by default, but run in CI.
While doing that we found that we needed to refactor the fakes to handle command line arguments which are not known up front; we do this by using regular expresseions.
Because the commits' messages say "Closes" and there was a plugin enabled globally (all k8s orgs) more or less recently which does not allow that. Those commits seem to have made it into the original repo, before that plugin was enabled.
Yeah, no idea why that is 🤷♂️ |
|
CLA problem is on 8c458a4 -- it's missing a GitHub user, according to the CLA bot. |
|
(GitHub isn't sure what to do with one "Josh Hill" either, so it looks like that's genuinely a problem) |
|
checking with contribex, looks like we'll have to file at ticket with the linuxfoundation. Will keep y'all posted |
|
Ok ... So Josh changed companies somewhen after that commit and therefore also changed his email address. I guess the CLA bot is now not able to connect that email address to a github or a linuxfoundation.org user. However it seems he had signed the CLA at some point, the original PR has the |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hoegaarden The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
It shouldn't void the CLA I think, but CNCF got back to me and (nicely) said "sorry, sucks to be you", so I think we'll need to replace the commit or 👻 force-merge 👻 . Let's get the rest of the issues out of the way (like the commit messages with "Fixes" / "Closes" in them) first. |
X-Closes #52
... and explicitly state that in the docs.
fixes bug when retrying controlPlane.Start() here: https://github.com/kubernetes-sigs/controller-runtime/blob/6101f6954a042342f1153c9b938813ad6995052c/pkg/envtest/server.go#L186
Co-Authored-By: s12chung <s12chung@users.noreply.github.com>
|
I can do that (rewording the offending commit messages). However, my approach with using I wonder if this will cause more pain in future than necessary and if we should just squash the commits and bring the thing in in one big |
|
Keywords which can automatically close issues and at(@) mentions are not allowed in commit messages. The list of commits with invalid commit messages: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
k8s-ci-robot
added
size/M
|
@hoegaarden: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Fix kustomize go link in quickstart
This brings in https://github.com/kubernetes-sigs/testing_frameworks/tree/master/integration (and all it's history) as discussed in #676.
The important commits are:
/assign @DirectXMan12 @mengqiy