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

hack/gotest.sh: Drop unnecessary script #135

Merged
merged 1 commit into from
Aug 15, 2018

Conversation

wking
Copy link
Member

@wking wking commented Aug 15, 2018

The script is from b8a9bbc (Remove bazel from test process, 2018-08-01, #97), as part of moving tests from Travis to Prow. But Prow is already running our Go unit tests, via pull-ci-origin-installer-unit with this config. It's had that setup since openshift/release@eb11aa8b (openshift/release#921), so we don't need to replicate it here.

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 15, 2018
@wking
Copy link
Member Author

wking commented Aug 15, 2018

/hold

I've got a WIP commit in here with a Go test violation to show Prow noticing the issue. Once we have that confirmed, I'll remove that WIP commit before we land this.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 15, 2018
@wking
Copy link
Member Author

wking commented Aug 15, 2018

/hold cancel

Here's the error:

2018/08/15 16:39:31 Executing test unit
ok  	github.com/openshift/installer/installer/pkg/config	0.072s
?   	github.com/openshift/installer/installer/pkg/config/aws	[no test files]
?   	github.com/openshift/installer/installer/pkg/config/libvirt	[no test files]
ok  	github.com/openshift/installer/installer/pkg/config-generator	0.442s
ok  	github.com/openshift/installer/installer/pkg/copy	0.028s
ok  	github.com/openshift/installer/installer/pkg/tls	1.718s
--- FAIL: TestInt (0.00s)
	validate_test.go:42: For Int("2 3"), expected "cannot be empty", got "invalid integer"
FAIL
FAIL	github.com/openshift/installer/installer/pkg/validate	1.702s
ok  	github.com/openshift/installer/installer/pkg/workflow	0.393s
2018/08/15 16:39:41 Container test in pod unit failed, exit code 1, reason Error
2018/08/15 16:39:41 Ran for 39s
error: could not run steps: failed to wait for test pod to complete: could not wait for pod completion: the pod ci-op-16sif3zj/unit failed after 9s (failed containers: test):  unknown

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 15, 2018
@wking wking force-pushed the drop-go-test-script branch from 19c49b6 to f4b5df8 Compare August 15, 2018 16:41
@sallyom
Copy link
Contributor

sallyom commented Aug 15, 2018

@wking thanks for taking care of this, indeed we do not need this script, we did not know of the ci-operator job when we originally picked up the task of converting all travis jobs to prow 👍

The script is from b8a9bbc (Remove bazel from test process,
2018-08-01, openshift#97), as part of moving tests from Travis to Prow.  But
Prow is already running our Go unit tests, via
pull-ci-origin-installer-unit [1] with this config [2].  It's had that
setup since openshift/release@eb11aa8b (Bump config to take advantage
of canonical setup, 2018-06-06, openshift/release#921), so we don't
need to replicate it here.

[1]: https://github.com/openshift/release/blob/6f623b15854ae91203243af22d7a259ab85acd86/ci-operator/jobs/openshift/installer/openshift-installer-presubmits.yaml#L3-L28
[2]: https://github.com/openshift/release/blob/6f623b15854ae91203243af22d7a259ab85acd86/ci-operator/config/openshift/installer/master.json#L44
@wking wking force-pushed the drop-go-test-script branch from f4b5df8 to a4187f6 Compare August 15, 2018 18:30
@wking
Copy link
Member Author

wking commented Aug 15, 2018

Rebased around #118 now that that's landed.

@crawford
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 15, 2018
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crawford, wking

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit f8f3934 into openshift:master Aug 15, 2018
@wking wking deleted the drop-go-test-script branch August 15, 2018 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants