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

fix: update vsphere-supervisor test to skip output from retry (follow-up) #323

Merged

Conversation

dilyar85
Copy link
Contributor

@dilyar85 dilyar85 commented Nov 9, 2023

This is a follow-up change to #321 which appears still failing the CI test.

To verify, I manually updated the test code to ensure the timing issue occurs:

--- a/builder/vsphere/supervisor/step_watch_source_test.go
+++ b/builder/vsphere/supervisor/step_watch_source_test.go
@@ -116,6 +116,8 @@ func TestWatchSource_Run(t *testing.T) {
        vmObj.Status.VmIp = testVMIP
        _ = kubeClient.Update(ctx, vmObj, opt)

+       time.Sleep(time.Second)
+
        vmServiceObj.Status.LoadBalancer.Ingress[0].IP = testIngressIP
        _ = kubeClient.Update(ctx, vmServiceObj, opt)

Running the above test against upstream/main:

$ go test -count=1 -run ^TestWatchSource_Run$ github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/supervisor
2023/11/09 10:38:35 ui: Waiting for the source VM to be powered-on and accessible...
2023/11/09 10:38:36 ui: Source VM is NOT powered-on yet, continue watching...
2023/11/09 10:38:36 ui: Source VM is powered-on, waiting for an IP to be assigned...
2023/11/09 10:38:36 ui: Successfully obtained the source VM IP: 1.2.3.4
2023/11/09 10:38:36 ui: Getting source VM ingress IP from the VMService object
2023/11/09 10:38:36 ui: VMService object's ingress IP is empty, continue checking...
2023/11/09 10:38:36 Retryable error: VMService object's ingress IP is empty, continue checking...
2023/11/09 10:38:41 ui: Successfully retrieved the source VM ingress IP: 5.6.7.8
2023/11/09 10:38:41 ui: Source VM is now ready in Supervisor cluster
--- FAIL: TestWatchSource_Run (6.00s)
    utils_test.go:61: Expected output "Successfully retrieved the source VM ingress IP: 5.6.7.8" but got "VMService object's ingress IP is empty, continue checking..."
FAIL
FAIL	github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/supervisor	6.748s
FAIL

Running the above test against this PR:

$ go test -count=1 -run ^TestWatchSource_Run$ github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/supervisor
ok  	github.com/hashicorp/packer-plugin-vsphere/builder/vsphere/supervisor	6.587s

@dilyar85 dilyar85 requested a review from a team as a code owner November 9, 2023 15:49
@@ -71,7 +71,7 @@ func readLine(t *testing.T, writer *bytes.Buffer) string {

// Skip "continue checking" line as it can be printed from the retry.
if strings.Contains(actual, "continue checking") {
readLine(t, writer)
return readLine(t, writer)
Copy link
Contributor

@nywilken nywilken Nov 9, 2023

Choose a reason for hiding this comment

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

Ha I was looking at this line in the previous PR and thought do we return properly here. Good catch.

Copy link
Contributor Author

@dilyar85 dilyar85 Nov 9, 2023

Choose a reason for hiding this comment

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

I think I just have some muscle memory that avoids recursions as much as possible :)

@tenthirtyam tenthirtyam added chore Chore builder/vsphere-supervisor Builder: vsphere-supervisor labels Nov 9, 2023
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

LGTM

@nywilken nywilken merged commit dfe0677 into hashicorp:main Nov 9, 2023
12 checks passed
@hashicorp hashicorp locked and limited conversation to collaborators Jun 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
builder/vsphere-supervisor Builder: vsphere-supervisor chore Chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants