[WIP] Introduce --namespace test flag and randomize test object names #3313

mgencur · 2020-06-11T18:10:52Z

This PR uses random names for test objects and makes it possible to use a common test namespace as opposed to a separate namespace for each test.

Fixes #2776

Proposed Changes

Reuse --namespace flag from knative.dev/pkg
Randomize test object names

Release Note

Docs

knative-prow-robot · 2020-06-11T18:11:04Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mgencur
To complete the pull request process, please assign chizhg
You can assign the PR to them by writing /assign @chizhg in a comment when ready.

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

Needs approval from an approver in each of these files:

test/OWNERS

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

mattmoor

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -type f -name '*.go' -print)

mgencur · 2020-06-11T18:34:28Z

/hold

I'd like to try running the test suite with the flag enabled.

grantr · 2020-06-11T19:07:52Z

/cc @lberk

mgencur · 2020-06-11T19:30:17Z

Hmm. I suppose limiting the length of namespace will require at least sub tests to have unique resource names so that they don't clash if they share the same namespace that is named according to the top-level test . Let's see.
Right now there's only one test (TestDefaultBrokerWithManyTriggers ) where subtests would share same namespace.

mgencur · 2020-06-11T20:05:07Z

Alright. Due to the length of some tests this is more complicated :-/ Right now, I'm not sure this is the right solution.

test/lib/test_runner.go

matzew · 2020-06-25T09:16:40Z

wanna move this forward?

mgencur · 2020-06-25T10:25:53Z

I'm working on this.

mattmoor

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
goimports -w $(find -name '*.go' | grep -v vendor | grep -v third_party | grep -v .pb.go | grep -v wire_gen.go)

mgencur · 2020-06-25T11:19:14Z

Hey, this is a preview of the new impl which uses a common namespace for all tests. I've converted three top-level tests to this new approach and they pass when running in parallel.
Please review before I move forward and modify the rest of the test suite to this style. Thanks!

mattmoor

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
goimports -w $(find -name '*.go' | grep -v vendor | grep -v third_party | grep -v .pb.go | grep -v wire_gen.go)

knative-test-reporter-robot · 2020-06-25T12:37:10Z

The following jobs failed:

Test name	Triggers	Retries
pull-knative-eventing-integration-tests		0/3

Failed non-flaky tests preventing automatic retry of pull-knative-eventing-integration-tests:

test/e2e.TestTriggerNoBroker
test/e2e.TestTriggerNoBroker/Channel-messaging.knative.dev/v1beta1
test/e2e.TestTriggerNoBroker/InMemoryChannel-messaging.knative.dev/v1beta1

knative-prow-robot · 2020-06-28T21:59:35Z

@mgencur: PR needs rebase.

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.

devguyio

left a nit comment, otherwise lgtm.

devguyio · 2020-07-13T12:27:44Z

test/e2e/helpers/broker_test_helper.go

 }

 // ToString converts the test case to a string to create names for different objects (e.g., triggers, services, etc.).
-func (tc eventTestCase) String() string {


It seems the comment on line 46 was forgotten to be updated?

lberk · 2020-07-14T02:31:11Z

test/e2e/helpers/broker_test_helper.go

@@ -245,7 +251,7 @@ func TestBrokerWithManyTriggers(t *testing.T, brokerCreator BrokerCreator, shoul
 			eventTrackers := make(map[string]*recordevents.EventInfoStore, len(test.eventFilters))
 			for _, event := range test.eventFilters {
 				// Create event recorder pod and service
-				subscriberName := "dumper-" + event.String()
+				subscriberName := event.Name()


Why did we remove the "dumper-" prefix (and "trigger-" one below)? Those common prefixes tend to help me establish the mental models when trying to debug a new (to me) test.

ninjaedit: maybe we can do something similar to your sender change? testlib.NameForTest("dumper-", even.Name()) ?

I removed it because the names need to be created in advance, and if I then prefix it with "dumper-" it may exceed the 63 limit, plus I'm getting segmentation faults because eventTrackers then don't include the right matcher and it calls a function on nil object. Anyway, I'll see if we can improve it a bit.

Right now I'm facing a problem with running tests in parallel. Some of them label namespaces with knative-eventing-injection": "enabled" and some of them don't want this. These tests can't run in parallel. Pretty complicated...

mgencur · 2020-07-24T07:19:45Z

I'm putting this on hold becuase there are plans to start a new architecture for the whole test suite soon. It does not make sense to go ahead and invest time in the old infra.

knative-prow-robot · 2020-07-28T20:32:46Z

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

Test name	Commit	Details	Rerun command
pull-knative-eventing-integration-tests	`108dfb3`	link	`/test pull-knative-eventing-integration-tests`
pull-knative-eventing-conformance-tests	`108dfb3`	link	`/test pull-knative-eventing-conformance-tests`

Full PR test history. Your PR dashboard.

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.

github-actions · 2020-10-27T01:34:18Z

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 11, 2020

knative-prow-robot requested review from chaodaiG and yt3liu June 11, 2020 18:11

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/test-and-release Test infrastructure, tests or release labels Jun 11, 2020

mattmoor reviewed Jun 11, 2020

View reviewed changes

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 11, 2020

knative-prow-robot requested a review from lberk June 11, 2020 19:07

mgencur force-pushed the stable_namespace_flag branch from 32b8b1e to ab68a85 Compare June 11, 2020 19:39

lberk reviewed Jun 11, 2020

View reviewed changes

test/lib/test_runner.go Outdated Show resolved Hide resolved

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 21, 2020

Use one test namespace

b85e4cd

mgencur force-pushed the stable_namespace_flag branch from ab68a85 to b85e4cd Compare June 25, 2020 11:15

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 25, 2020

mgencur changed the title ~~Introduce --stablenamespaces test flag~~ Introduce --namespace test flag and randomize test object names Jun 25, 2020

mattmoor reviewed Jun 25, 2020

View reviewed changes

mgencur changed the title ~~Introduce --namespace test flag and randomize test object names~~ [WIP] Introduce --namespace test flag and randomize test object names Jun 25, 2020

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 25, 2020

mgencur added 2 commits June 25, 2020 13:24

Fix formatting

4286dfb

Fix after rebase

864aba9

mattmoor reviewed Jun 25, 2020

View reviewed changes

Fix format

108dfb3

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 28, 2020

devguyio reviewed Jul 13, 2020

View reviewed changes

lberk reviewed Jul 14, 2020

View reviewed changes

lberk mentioned this pull request Sep 1, 2020

Add lberk to approvers #3165

Merged

github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 27, 2020

github-actions bot closed this Nov 27, 2020

devguyio mentioned this pull request Jan 7, 2021

Add devguyio to approvers #4700

Closed

mgencur mentioned this pull request Apr 29, 2021

Allow for tests to run in a pre-created NS #2776

Closed

mgencur mentioned this pull request May 12, 2021

Use test namespaces with numeric suffix #5337

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Introduce --namespace test flag and randomize test object names #3313

[WIP] Introduce --namespace test flag and randomize test object names #3313

mgencur commented Jun 11, 2020 •

edited

Loading

knative-prow-robot commented Jun 11, 2020

mattmoor left a comment

mgencur commented Jun 11, 2020 •

edited

Loading

grantr commented Jun 11, 2020

mgencur commented Jun 11, 2020 •

edited

Loading

mgencur commented Jun 11, 2020 •

edited

Loading

matzew commented Jun 25, 2020

mgencur commented Jun 25, 2020

mattmoor left a comment

mgencur commented Jun 25, 2020

mattmoor left a comment

knative-test-reporter-robot commented Jun 25, 2020

knative-prow-robot commented Jun 28, 2020

devguyio left a comment

devguyio Jul 13, 2020

lberk Jul 14, 2020 •

edited

Loading

mgencur Jul 23, 2020

mgencur Jul 23, 2020

mgencur commented Jul 24, 2020

knative-prow-robot commented Jul 28, 2020

github-actions bot commented Oct 27, 2020

[WIP] Introduce --namespace test flag and randomize test object names #3313

[WIP] Introduce --namespace test flag and randomize test object names #3313

Conversation

mgencur commented Jun 11, 2020 • edited Loading

Proposed Changes

knative-prow-robot commented Jun 11, 2020

mattmoor left a comment

Choose a reason for hiding this comment

mgencur commented Jun 11, 2020 • edited Loading

grantr commented Jun 11, 2020

mgencur commented Jun 11, 2020 • edited Loading

mgencur commented Jun 11, 2020 • edited Loading

matzew commented Jun 25, 2020

mgencur commented Jun 25, 2020

mattmoor left a comment

Choose a reason for hiding this comment

mgencur commented Jun 25, 2020

mattmoor left a comment

Choose a reason for hiding this comment

knative-test-reporter-robot commented Jun 25, 2020

knative-prow-robot commented Jun 28, 2020

devguyio left a comment

Choose a reason for hiding this comment

devguyio Jul 13, 2020

Choose a reason for hiding this comment

lberk Jul 14, 2020 • edited Loading

Choose a reason for hiding this comment

mgencur Jul 23, 2020

Choose a reason for hiding this comment

mgencur Jul 23, 2020

Choose a reason for hiding this comment

mgencur commented Jul 24, 2020

knative-prow-robot commented Jul 28, 2020

github-actions bot commented Oct 27, 2020

mgencur commented Jun 11, 2020 •

edited

Loading

mgencur commented Jun 11, 2020 •

edited

Loading

mgencur commented Jun 11, 2020 •

edited

Loading

mgencur commented Jun 11, 2020 •

edited

Loading

lberk Jul 14, 2020 •

edited

Loading