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

test: Remove duplicate test cases from K8sServicesTest #11523

Merged
merged 1 commit into from May 14, 2020

Conversation

brb
Copy link
Member

@brb brb commented May 13, 2020

This commit removes two test cases ("TC + DR + {DSR,SNAT}") as they are duplicated by the tests in "Tests with direct routing".

This commit removes two test cases ("TC + DR + {DSR,SNAT}") as they
duplicated by the tests in "Tests with direct routing".

Signed-off-by: Martynas Pumputis <m@lambda.lt>
@brb brb added area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/misc This PR makes changes that have no direct user impact. labels May 13, 2020
@brb brb requested a review from borkmann May 13, 2020 17:18
@brb brb requested a review from a team as a code owner May 13, 2020 17:18
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 May 13, 2020
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 38.008% when pulling 4985421 on pr/brb/reduce-svc-tests into a7ec82e on master.

@brb
Copy link
Member Author

brb commented May 14, 2020

test-me-please

@brb brb changed the title test: Remove some test cases from K8sServicesTest test: Remove duplicate test cases from K8sServicesTest May 14, 2020
@brb
Copy link
Member Author

brb commented May 14, 2020

retest-net-next

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 14, 2020
@@ -1252,6 +1253,7 @@ var _ = Describe("K8sServicesTest", func() {
})

testDSR(64001)
testNodePort(true, false, false) // no need to test from outside, as testDSR did it
Copy link
Member

Choose a reason for hiding this comment

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

The previous tests also cleared the CT tables on the node after testNodePort(), is that handled here somehow or not necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I can see, it's not necessary. I haven't seen a failure due to potentially stale CT entries.

Copy link
Member

Choose a reason for hiding this comment

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

Good enough for me, we can keep an eye out in case this concern is warranted.

@joestringer joestringer merged commit ffe9cb2 into master May 14, 2020
1.8.0 automation moved this from In progress to Merged May 14, 2020
@joestringer joestringer deleted the pr/brb/reduce-svc-tests branch May 14, 2020 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

5 participants