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

internal/daemon/worker: speed up multi-worker tests #5370

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

johanbrandhorst
Copy link
Collaborator

@johanbrandhorst johanbrandhorst commented Dec 17, 2024

Instead of sleeping, we can poll for the desired state. Also, run shutdowns in parallel. This mostly affects the enterprise tests that start multiple workers.

Instead of sleeping, we can poll for the desired state. Also,
run shutdowns in parallel.
require.Eventually(
func() bool {
t.Log("Checking worker status")
workers, err := serversRepo.ListWorkers(controllerContext, []string{"global"})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: Is NewTestMultihopWorkers ever called by parallel tests? Could this impact the output of ListWorkers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not as long as each test is using it's own controller, which I think we have to assume the parallel tests are doing.

Copy link
Member

@tmessi tmessi left a comment

Choose a reason for hiding this comment

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

Nice!

@johanbrandhorst johanbrandhorst merged commit 862a30c into main Dec 17, 2024
60 of 63 checks passed
@johanbrandhorst johanbrandhorst deleted the jbrandhorst-speed-up-multi-worker-tests branch December 17, 2024 17:18
johanbrandhorst added a commit that referenced this pull request Dec 19, 2024
Unfortunately, this started causing the enterprise tests to break in
an unexpected way. Reverting to spend some more time testing the changes.
johanbrandhorst added a commit that referenced this pull request Dec 19, 2024
Unfortunately, this started causing the enterprise tests to break in
an unexpected way. Reverting to spend some more time testing the changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants