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

test: fix flaky test TestOpenWorkspaceFromPrebuild #13170

Merged
merged 1 commit into from
Sep 23, 2022
Merged

test: fix flaky test TestOpenWorkspaceFromPrebuild #13170

merged 1 commit into from
Sep 23, 2022

Conversation

jenting
Copy link
Contributor

@jenting jenting commented Sep 21, 2022 •
edited
Loading

Description

  • Fall back to check the init task message and file exits
  • Check the files and folders permission under .git/ is not root

Related Issue(s)

Related #12638

How to test

cd test

go test  -run ^TestOpenWorkspaceFromPrebuild$ github.com/gitpod-io/gitpod/test/tests/components/ws-manager -count=1 -namespace=default -kubeconfig=/home/gitpod/.kube/config -v -timeout=20m

Release Notes

None

Documentation

None

Werft options:

  • /werft with-preview
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide

@jenting jenting marked this pull request as ready for review September 21, 2022 23:24
@jenting jenting requested a review from a team September 21, 2022 23:24
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label Sep 21, 2022
@utam0k
Copy link
Contributor

utam0k commented Sep 21, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-integration-tests=workspace

👍 started the job as gitpod-build-jenting-integration-tests-add-prebuilds-12638.12
(with .werft/ from main)

@jenting jenting mentioned this pull request Sep 22, 2022
Merged
2 tasks
@jenting jenting force-pushed the jenting/integration-tests-add-prebuilds-12638 branch from cd9a3c2 to 1ae3708 Compare September 22, 2022 01:34
@jenting
Copy link
Contributor Author

jenting commented Sep 22, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-integration-tests=workspace

👍 started the job as gitpod-build-jenting-integration-tests-add-prebuilds-12638.14
(with .werft/ from main)

@roboquat roboquat added size/L and removed size/M labels Sep 22, 2022
@utam0k
Copy link
Contributor

utam0k commented Sep 22, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-integration-tests=workspace wit-preview

👍 started the job as gitpod-build-jenting-integration-tests-add-prebuilds-12638.15
(with .werft/ from main)

@utam0k
Copy link
Contributor

utam0k commented Sep 22, 2022

@jenting We need to with-preview when you want to run with-integration

@jenting
Copy link
Contributor Author

jenting commented Sep 22, 2022 •
edited
Loading

with-preview

But I have the preview env already. And this is only a test cases change. Should I still need the with-preview flag?

@utam0k
Copy link
Contributor

utam0k commented Sep 22, 2022

But I have the preview env already. And this is only a test cases change. Should I still need the with-preview flag?

AFAI, yes, we need.

@jenting
Copy link
Contributor Author

jenting commented Sep 22, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-integration-tests=workspace with-preview=true

👍 started the job as gitpod-build-jenting-integration-tests-add-prebuilds-12638.16
(with .werft/ from main)

@jenting jenting changed the title test: fall back to check the init task message and file exits test: fix flaky test TestOpenWorkspaceFromPrebuild Sep 22, 2022
utam0k
utam0k reviewed Sep 22, 2022
View reviewed changes
test/tests/components/ws-manager/prebuild_test.go Outdated
Comment on lines 495 to 505
if lastStatus == nil || lastStatus.Conditions == nil || lastStatus.Conditions.VolumeSnapshot == nil {
return "", nil, errors.New("cannot find the last snapshot")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the root cause for flakey tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops sorry, I wanted to ask if this code solved flaky.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Several reasons for the flaky test happens, but the primary reason is that the prebuild log without the message 🤙 This task ran as a workspace prebuild in /workspace/.gitpod/prebuild-log-*. This message is written by the supervisor when running the prebuild workspace. Somehow, the message does not write into it. I increased the init task with 90 seconds of sleep and hope this will improve the message 🤙 This task ran as a workspace prebuild be written into the /workspace/.gitpod/prebuild-log-*.

If the above issue happens, we fall back to checking the init task message within /workspace/.gitpod/prebuild-log-*.

If it still fails, we fall back to checking the someFile generated by the init task exists, this makes sure the workspace pod is created from prebuild, not from the git clone.

@jenting jenting force-pushed the jenting/integration-tests-add-prebuilds-12638 branch from 1ae3708 to ce956ec Compare September 22, 2022 04:25
@jenting jenting marked this pull request as draft September 22, 2022 04:29
@jenting jenting force-pushed the jenting/integration-tests-add-prebuilds-12638 branch from ce956ec to f9633e9 Compare September 22, 2022 05:00
@jenting jenting force-pushed the jenting/integration-tests-add-prebuilds-12638 branch 2 times, most recently from 38e40fd to 2dfa983 Compare September 22, 2022 09:50
@jenting jenting marked this pull request as ready for review September 22, 2022 12:51
@jenting jenting requested a review from utam0k September 22, 2022 12:51
@jenting
Copy link
Contributor Author

jenting commented Sep 22, 2022

Three test results

jenting
jenting commented Sep 22, 2022
View reviewed changes
test/tests/components/ws-manager/prebuild_test.go
Comment on lines +186 to +198
// TestOpenWorkspaceFromPrebuild
// - create a prebuild
// - open the workspace from prebuild
// - make sure the .git/ folder with correct permission
// - make sure either one of the condition mets
// - the prebuild log message exists
// - the init task message exists
// - the init task generated file exists
//
// - write a new file foobar.txt
// - stop the workspace
// - relaunch the workspace
// - make sure the file foobar.txt exists
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add the test steps here, so we could more easily understand the test steps without tracing the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great!

@jenting jenting requested a review from a team September 23, 2022 01:26
@roboquat roboquat merged commit 41c034c into main Sep 23, 2022
@roboquat roboquat deleted the jenting/integration-tests-add-prebuilds-12638 branch September 23, 2022 06:35
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@utam0k utam0k utam0k approved these changes

Assignees
No one assigned
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note-none size/L team: workspace Issue belongs to the Workspace team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jenting @utam0k @roboquat