Allow running a subset of the integration tests #16153
Conversation
openshift-ci-robot
added
the
size/M
|
@stevekuznetsov my BASH hacks are probably wrong... |
|
|
||
| var timeout = flag.Duration("sub.timeout", 0, "Specify the timeout for each sub test") | ||
| var retries = flag.Int("sub.retries", 1, "Specify the flake retries for each sub test") | ||
| var run = flag.String("sub.run", ".*", "Run only those sub tests matching the regular expression") |
There was a problem hiding this comment.
Don't do this. All the sub tests are TestIntegration/*. Use that.
hack/test-integration.sh
Outdated
| os::util::environment::update_path_var | ||
|
|
||
| COVERAGE_SPEC=" " DETECT_RACES=false TMPDIR="${BASETMPDIR}" TIMEOUT=45m GOTESTFLAGS="-sub.timeout=120s" "${OS_ROOT}/hack/test-go.sh" "test/integration/runner" | ||
| COVERAGE_SPEC=" " DETECT_RACES=false TMPDIR="${BASETMPDIR}" TIMEOUT=45m GOTESTBINARY_FLAGS="-sub.timeout=120s -sub.run=${1:-}" "${OS_ROOT}/hack/test-go.sh" "test/integration/runner" |
There was a problem hiding this comment.
Pass the flag to test-go, not to the sub run.
There was a problem hiding this comment.
+1, ${1:+-run TestIntegration/}${1:-}
hack/test-go.sh
Outdated
| coverage_output_dir="${COVERAGE_OUTPUT_DIR:-}" | ||
| coverage_spec="${COVERAGE_SPEC:--cover -covermode atomic}" | ||
| gotest_flags="${GOTEST_FLAGS:-}" | ||
| gotestbinary_flags="${GOTESTBINARY_FLAGS:-}" |
There was a problem hiding this comment.
why isn't this gotestflags?
There was a problem hiding this comment.
This is my only comment Bash-wise
There was a problem hiding this comment.
The current file in master has a typo GOTESTFLAGS vs GOTEST_FLAGS. If you correct the typo, the runner breaks because you cannot pass arguments to a test binary without using -args .... So I do not understand how to pass arguments (sub.timeout) to test-go and somehow have the test runner honor that.
GOTEST_FLAGS="-v -run TestIntegration/${1:-.*}" works.
These do not work:
"-v -run TestIntegration/${1:-.*}" -args -sub.timeout=120s"
"-v -run TestIntegration/${1:-.*}" -sub.timeout=120s"
There was a problem hiding this comment.
The command-line package list, if present, must appear before any
flag not known to the go test command. Continuing the example above,
the package list would have to appear before -myflag, but could appear
on either side of -v.
—fromgo test -help
test-go.sh uses GOTEST_FLAGS before a package name, therefore -sub.timeout=120s cannot be used in GOTEST_FLAGS. On the other side, we can try to change the order of arguments in test-go.sh.
enj
force-pushed
the
enj/t/integration_run_regex
branch
from
194fbc4 to
7792806
Compare
enj
changed the title
|
@smarterclayton @stevekuznetsov @dmage PTAL. I went with the smallest change that allows running a subset of integration tests and does not further complicate |
This change restores the previous behavior of running `hack/test-integration.sh <regex>`. The timeout flag is no longer passed to the integration test runner. To do so correctly requires using `go test -args` which would add further complexity to `hack/test-go.sh`. Signed-off-by: Monis Khan <mkhan@redhat.com>
enj
force-pushed
the
enj/t/integration_run_regex
branch
from
7792806 to
d4a566c
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enj, stevekuznetsov The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-merge-robot
added
the
approved
|
Automatic merge from submit-queue (batch tested with PRs 16226, 16377, 15766, 16299, 16153) |
6 participants
This change restores the previous behavior of running
hack/test-integration.sh <regex>. The timeout flag is no longer passed to the integration test runner. To do so correctly requires usinggo test -argswhich would add further complexity tohack/test-go.sh.Signed-off-by: Monis Khan mkhan@redhat.com
/assign @smarterclayton