TestFunctional/parallel/TunnelCmd into serial subtest #7956
Conversation
k8s-ci-robot
added
kind/cleanup
|
Hi @govargo. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
k8s-ci-robot
added
the
needs-ok-to-test
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: govargo The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/M
|
/assign @priyawadhwa |
|
Can one of the admins verify this patch? |
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
kvm2 Driver |
|
kvm2 Driver Times for Minikube (PR 7956): [67.16044052800001 64.244560162 66.422675896] Averages Time Per Log docker Driver Times for Minikube (PR 7956): [26.673651135 27.172496464999995 26.077300579999996] Averages Time Per Log |
|
/retest |
Travis tests have failedHey @govargo, TravisBuddy Request Identifier: 7b304550-8c1e-11ea-bf8a-61073f267ac6 |
|
I saw this failure first time... /retest |
|
kvm2 Driver Times for Minikube (PR 7956): [63.84015205000001 66.40918797 64.89939340699999] Averages Time Per Log docker Driver Times for Minikube (PR 7956): [28.006389807000005 27.175415786 26.786381156000004] Averages Time Per Log |
test/integration/fn_tunnel_cmd.go
Outdated
| } | ||
| dnsIP := "@" + ip.String() | ||
|
|
||
| // Test.2-1 DNS resolution by dig (experimental): https://minikube.sigs.k8s.io/docs/handbook/accessing/#dns-resolution-experimental |
There was a problem hiding this comment.
instead of adding comment test1
please use a t.Run("serial"){ } with 3 subtests, that run in a for loop. (to make sure they are in order. like this test
minikube/test/integration/pause_test.go
Line 38 in 100ba9b
There was a problem hiding this comment.
Thanks for your advice!
I'll look it.
k8s-ci-robot
added
size/L
Codecov Report
@@ Coverage Diff @@
## master #7956 +/- ##
==========================================
- Coverage 36.04% 35.78% -0.27%
==========================================
Files 144 143 -1
Lines 9196 9174 -22
==========================================
- Hits 3315 3283 -32
- Misses 5480 5490 +10
Partials 401 401
|
|
@medyagh So I made the tests independent such as I don't understand E2E integration test scripts at all. |
|
kvm2 Driver Times for Minikube (PR 7956): [65.154739951 65.058502576 64.455458615] Averages Time Per Log docker Driver Times for Minikube (PR 7956): [27.821524239000002 26.020596425999997 26.618272315] Averages Time Per Log |
|
/retest |
k8s-ci-robot
removed
the
size/L
|
kvm2 Driver Times for Minikube (PR 7956): [64.192190425 66.647977383 67.25618308899998] Averages Time Per Log docker Driver Times for Minikube (PR 7956): [27.988238541 28.051676789 28.168322204000003] Averages Time Per Log |
govargo
added
the
do-not-merge/work-in-progress
|
After trying source change, only But other tests on other OS are success.
Does anyone have any ideas why is macOS test failure? (macOS jenkins flake? Or need to set PROXY env?) |
|
/retest |
k8s-ci-robot
added
size/XL
|
kvm2 Driver Times for Minikube (PR 7956): [65.281246283 66.44230932800001 63.993592288] Averages Time Per Log docker Driver Times for Minikube (PR 7956): [28.441599499999995 28.603291416999994 28.586532047] Averages Time Per Log |
test/integration/functional_test.go
Outdated
| @@ -124,7 +124,6 @@ func TestFunctional(t *testing.T) { | |||
| {"ServiceCmd", validateServiceCmd}, | |||
| {"AddonsCmd", validateAddonsCmd}, | |||
| {"PersistentVolumeClaim", validatePersistentVolumeClaim}, | |||
| {"TunnelCmd", validateTunnelCmd}, | |||
There was a problem hiding this comment.
I think tunnel test should still be part of Functional Tests, since we consider this test as a Core Test that tests all functionalities of minikube.
when I meant to do it in serial, I meant tunnel test itself runs in parallel but all of it is own sub tests run in serial.
There was a problem hiding this comment.
@medyagh
First time I tried to add the serial test into validateTunnelCmd , but the subtests failed because the process stopped like a deadlock then.
After your this comment, I tried to add serial test into validateTunnelCmd of TestFunctional again.
And now serial tunnel tests works and the HyperKit_macOS test all successed!!! 💯
k8s-ci-robot
added
size/L
|
kvm2 Driver Times for Minikube (PR 7956): [65.41044705099999 64.529045405 67.00706772199999] Averages Time Per Log docker Driver Times for Minikube (PR 7956): [31.624924904000004 28.928593534999997 28.407475956] Averages Time Per Log |
govargo
removed
the
do-not-merge/work-in-progress
govargo
changed the title
What type of PR is this?
/kind cleanup
/area testing
/kind failing-test
What this PR does / why we need it:
This is improvement & deep investigation PR for #7809.
Now, Hyperkit_macOS integration test always fails due to not be able to resolving DNS.
DNS resolution works in local macOS. So I think root cause of fail is releated to Jenkins environment or test server's environment.
This PR splits
TestFunctional/parallel/TunnelCmdand adds DNS resolution test step.And if DNS resolution test step is failed, we can get
digcommand output.This will be helpful for deep investigation.
(EDIT)
This PR changed test
validateTunnelCmdofTestFunctionalin order to make sure test execution serially.Which issue(s) this PR fixes:
This will be helpful for #7809 and confirmed the issue fixed.
https://storage.googleapis.com/minikube-builds/logs/7956/884f0e2/HyperKit_macOS.html#pass_TestFunctional%2fparallel%2fTunnelCmd%2fserial%2fAccessThroughDNS
Does this PR introduce a user-facing change?
No. This PR changes
TestFunctional/parallel/TunnelCmdtestcode.Before this PR
Test
validateTunnelCmdis tested inTestFunctional.After this PR
validateTunnelCmdhas serial subtests.Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: