Run the e2e tests in parallel by srinivashegde86 · Pull Request #1670 · knative/serving

srinivashegde86 · 2018-07-24T21:48:42Z

Part of knative/test-infra#20

The e2e tests take ~10 minutes to run. With this change, the e2e tests ~7 mintues.

Proposed Changes

Run the e2e tests in parallel. Run upto 4 test methods in parallel for now. This can be adjusted later, if we need.
Update the e2e tests to be marked as running in parallel except autoscaler e2e.
Mark the autoscaler e2e as to not run in parallel, as it was taking more time when running in parallel.
Update docs to include the new parallel flag

google-prow-robot · 2018-07-24T21:48:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: srinivashegde86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~test/OWNERS~~ [srinivashegde86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

srinivashegde86 · 2018-07-24T21:49:03Z

/hold need to verify how much time prow takes to run e2e.

srinivashegde86 · 2018-07-24T21:49:19Z

/hold

adrcunha · 2018-07-24T21:52:44Z

 ```bash
 go test -v -tags=e2e -count=1 ./test/conformance
-go test -v -tags=e2e -count=1 ./test/e2e
+go test -v -tags=e2e -count=1 -parallel=4 ./test/e2e


Running tests in parallel should be optional. I suggest you mention it in the docs, but avoid repeating it every time.

adrcunha · 2018-07-24T21:54:47Z

  local options=""
  (( EMIT_METRICS )) && options="-emitmetrics"
-  report_go_test -v -tags=e2e -count=1 ./test/$1 -dockerrepo gcr.io/knative-tests/test-images/$1 ${options}
+  if [ "$1" =  "e2e" ]; then


I confess I'm not comfortable with this hardcoded approach, unless it proves to be way simpler and faster than properly and transparently making the tests able to run in parallel...

report_go_test can determine whether it's running e2e or not based on input parameters, right?
In general, We can have an effective mode (say via a test flag). When it's in effective mode, test infra will be smart enough to run certain things in parallel. When it's not, everything is run sequentially. By default, we run effective mode.

Yes. The first param is e2e or conformance, and we can check that, as I have done here.

Running things in parallel can cause race conditions and add flakiness in the tests. This is not significantly faster(few minutes faster) than running them sequentially atleast for now as the tests other than autoscaler take max 1 min.

report_go_test is test-type agnostic. Anyway, this would just move the hack to a different place (that's even worse, because the function is meant to be repo-agnostic).

adrcunha · 2018-07-24T21:55:28Z

 	}

+	if parallel {
+		t.Parallel()


Yeah, one shouldn't need to figure out what tests can and what tests cannot run in parallel.

srinivashegde86 · 2018-07-24T22:47:14Z

/test pull-knative-serving-integration-tests

srinivashegde86 · 2018-07-25T05:01:44Z

/retest

google-prow-robot · 2018-07-25T05:31:33Z

@srinivashegde86: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-knative-serving-integration-tests	`b647049`	link	`/test pull-knative-serving-integration-tests`

Full PR test history. Your PR dashboard.

Details

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.

tcnghia · 2018-07-25T15:42:32Z

There is currently an issue with running tests in parallel: #1376 . Istio version 0.8 has a in bug in batching multiple route changes happening shortly one after another. The batching bug result in the route taking O(5 minutes) to take effect.

This issue is fixed in 1.0. We should wait for 1.0 to release and use that before checking in this change.

tcnghia · 2018-07-26T21:56:00Z

#1376 is fixed, should no longer be a blocker for running tests in parallel

googlebot · 2018-08-04T22:17:06Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

knative-prow-robot · 2018-08-04T23:08:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: srinivashegde86

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~test/OWNERS~~ [srinivashegde86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

googlebot · 2018-08-04T23:08:40Z

CLAs look good, thanks!

knative-prow-robot · 2018-08-04T23:26:49Z

@srinivashegde86: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-knative-serving-build-tests	`10db5c0`	link	`/test pull-knative-serving-build-tests`
pull-knative-serving-integration-tests	`10db5c0`	link	`/test pull-knative-serving-integration-tests`

Full PR test history. Your PR dashboard.

Details

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.

markusthoemmes · 2018-10-04T11:04:38Z

/cc @josephburnett

I think it's not even possible as of today to have our tests run reliably in parallel, given we do global changes? Does #2144 make that possible?

Run the e2e tests in parallel

b647049

google-prow-robot requested review from adrcunha and jessiezcc July 24, 2018 21:48

google-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 24, 2018

google-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2018

google-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 24, 2018

adrcunha suggested changes Jul 24, 2018

View reviewed changes

sukhil-suresh mentioned this pull request Jul 25, 2018

Initial PR for avoiding http retries by activator #1665

Closed

Run e2e tests in parallel

585abae

knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 4, 2018

Merge remote-tracking branch 'upstream/master' into integrationTests

10db5c0

knative-prow-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 4, 2018

knative-prow-robot requested a review from josephburnett October 4, 2018 11:04

srinivashegde86 closed this Oct 16, 2018

srinivashegde86 deleted the integrationTests branch October 16, 2018 17:35

Conversation

srinivashegde86 commented Jul 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Uh oh!

google-prow-robot commented Jul 24, 2018

Uh oh!

srinivashegde86 commented Jul 24, 2018

Uh oh!

srinivashegde86 commented Jul 24, 2018

Uh oh!

adrcunha Jul 24, 2018

Choose a reason for hiding this comment

Uh oh!

adrcunha Jul 24, 2018

Choose a reason for hiding this comment

Uh oh!

jessiezcc Jul 24, 2018

Choose a reason for hiding this comment

Uh oh!

srinivashegde86 Jul 24, 2018

Choose a reason for hiding this comment

Uh oh!

adrcunha Jul 24, 2018

Choose a reason for hiding this comment

Uh oh!

adrcunha Jul 24, 2018

Choose a reason for hiding this comment

Uh oh!

srinivashegde86 commented Jul 24, 2018

Uh oh!

srinivashegde86 commented Jul 25, 2018

Uh oh!

google-prow-robot commented Jul 25, 2018

Uh oh!

tcnghia commented Jul 25, 2018

Uh oh!

tcnghia commented Jul 26, 2018

Uh oh!

googlebot commented Aug 4, 2018

Uh oh!

knative-prow-robot commented Aug 4, 2018

Uh oh!

googlebot commented Aug 4, 2018

Uh oh!

knative-prow-robot commented Aug 4, 2018

Uh oh!

markusthoemmes commented Oct 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

srinivashegde86 commented Jul 24, 2018 •

edited

Loading