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.

Sign up for GitHub

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

Jump to bottom

[CI] Randomize ns in policy tests #10180

Merged
merged 1 commit into from Feb 19, 2020
Merged

[CI] Randomize ns in policy tests #10180

merged 1 commit into from Feb 19, 2020

Conversation

nebril
Copy link
Member

@nebril nebril commented Feb 13, 2020 •
edited by tgraf

This change is Reviewable

@nebril nebril requested a review from a team as a code owner February 13, 2020 15:11
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Feb 13, 2020
@nebril
Copy link
Member Author

nebril commented Feb 13, 2020

test-gke K8sPolicyTest*

@coveralls
Copy link

coveralls commented Feb 13, 2020 •
edited

Coverage Status

Coverage increased (+0.01%) to 44.545% when pulling 31392e3 on pr/fix-policy-tests-ns into ce71fc1 on master.

@nebril nebril added release-note/ci This PR makes changes to the CI. wip labels Feb 13, 2020
@nebril
Copy link
Member Author

nebril commented Feb 13, 2020

test-gke K8sPolicyTest*

@nebril nebril added pending-review and removed wip labels Feb 13, 2020
@nebril
Copy link
Member Author

nebril commented Feb 13, 2020

test-gke K8sPolicyTest*

1 similar comment
@nebril
Copy link
Member Author

nebril commented Feb 14, 2020

test-gke K8sPolicyTest*

raybejjani
raybejjani suggested changes Feb 14, 2020
View reviewed changes
Copy link
Contributor

@raybejjani raybejjani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically LGTM. Some small questions but once you add the comments then we can merge if you want.

test/helpers/kubectl.go Outdated Show resolved Hide resolved
test/helpers/kubectl.go Show resolved Hide resolved
test/k8sT/Policies.go
@@ -1336,8 +1337,8 @@ EOF`, k, v)
demoPath string
demoManifestNS1 string
demoManifestNS2 string
firstNS = "first"
secondNS = "second"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to call helpers.GenerateNamespaceForTest from here, no? Or are you worried that the timestamp will be wrong? (Would it matter, though? it just needs to be different than another test running and ginkgo will run the tests one-by-one no matter what)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running it here causes a segfault because ginkgo.CurrentGinkgoTestDescription() returns nil when run in Context.

test/k8sT/Policies.go
@@ -187,7 +187,7 @@ var _ = Describe("K8sPolicyTest", func() {
}

BeforeAll(func() {
namespaceForTest = helpers.GenerateNamespaceForTest()
namespaceForTest = helpers.GenerateNamespaceForTest("")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the seed can be generated once at the beginning of all tests? Then it would definitely be unique between runs and callers don't need to remember which number to use in the seed if they change the tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The seed is for making sure that if we have two namespaces (like in Clusterwide policies context), we are sure that generated names will be different.

@nebril
Copy link
Member Author

nebril commented Feb 14, 2020

test-gke K8sPolicyTest*

@nebril
Copy link
Member Author

nebril commented Feb 14, 2020

test-me-please

@nebril
Copy link
Member Author

nebril commented Feb 17, 2020

test-me-please

2 similar comments
@nebril
Copy link
Member Author

nebril commented Feb 17, 2020

test-me-please

@nebril
Copy link
Member Author

nebril commented Feb 17, 2020

test-me-please

@nebril
Copy link
Member Author

nebril commented Feb 18, 2020

test-focus K8sPolicyTest*

@aanm aanm added this to the 1.8 milestone Feb 18, 2020
@nebril
Copy link
Member Author

nebril commented Feb 19, 2020

test-me-please

@nebril
Copy link
Member Author

nebril commented Feb 19, 2020

test-gke K8sPolicyTest*

@nebril
[CI] Randomize ns in policy tests
31392e3 
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@nebril
Copy link
Member Author

nebril commented Feb 19, 2020

test-gke K8sPolicyTest*

@nebril
Copy link
Member Author

nebril commented Feb 19, 2020

test-focus K8sPolicyTest*

@raybejjani raybejjani merged commit ae9619a into master Feb 19, 2020
1.8.0 automation moved this from In progress to Merged Feb 19, 2020
@raybejjani raybejjani deleted the pr/fix-policy-tests-ns branch February 19, 2020 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raybejjani raybejjani raybejjani approved these changes

@aanm aanm aanm approved these changes

Assignees
No one assigned
Labels
release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.8.0
  
Merged
Milestone
1.8
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nebril @coveralls @raybejjani @aanm