Switch to ginkgo/gomega for e2e testing. #130
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
/cc @yanirq |
openshift-ci-robot
added
the
approved
| @@ -70,7 +70,9 @@ $(GOBINDATA_BIN): | |||
| $(GO) build -o $(GOBINDATA_BIN) ./vendor/github.com/kevinburke/go-bindata/go-bindata | |||
|
|
|||
| test-e2e: $(BINDATA) | |||
| KUBERNETES_CONFIG="$(KUBECONFIG)" $(GO) test -v -timeout 20m -tags e2e ./test/e2e/... -run . | |||
| for d in basic reboots; do \ | |||
There was a problem hiding this comment.
may i recommend a script running the suits recursively and add color setting selectively?
see https://github.com/openshift-kni/performance-addon-operators/blob/master/hack/run-functests.sh
There was a problem hiding this comment.
I can see some value, however adding a hack directory with random scripts is something I managed to avoid so far. I'd also like to avoid pulling extra dependencies like github.com/onsi/ginkgo/ginkgo which are not necessarily needed. If they really are needed, they'd have to be vendored in as from some build environments you cannot fetch external dependencies. As for vendoring in, I tend to follow "vendor in only things that are essential". Can you elaborate whether that dependency is essential?
As for running tests recursively, I wanted to enforce the ordering: basic tests first, advanced (requiring reboots) next. Also having the flexibility to enable/disable tests seemed handy.
jmencak
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
Changes:
- removed ginkgo/gomega and other dot imports
- added a test case for kernel parameter removal
- check for kernel parameter equality with optional exclusion
of selected parameters
|
Getting very close to the final state I believe. |
|
I like it! |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
| ginkgo.It(fmt.Sprintf("%s set", sysctlVar), func() { | ||
| ginkgo.By("getting a list of worker nodes") | ||
| nodes, err := util.GetNodesByRole(cs, "worker") | ||
| gomega.Expect(err).NotTo(gomega.HaveOccurred()) |
There was a problem hiding this comment.
is there something we can do with this gomega prefix while keeping the dot imports guidelines ? its a built bulky.
There was a problem hiding this comment.
If you look upstream k8s or OpenShift tests, none of them use dot imports as dot imports are considered bad practice. Also, I'm concerned about a possible removal of dot imports in future golang 2.x: golang/go#29326
- Replaced exec.Command by util.ExecAndLogCommand for better logging. - Added test scenario for adding kernel parameters in child profile.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jmencak, skordas, yanirq The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
Switching to ginkgo/gomega for e2e tests.