Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: avoid sharingecho.Responses across tests#17211

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ethanndickson merged 2 commits intomainfromethan/fix-echo-flakes
Apr 2, 2025

Conversation

ethanndickson
Copy link
Member

@ethanndicksonethanndickson commentedApr 2, 2025
edited
Loading

Closescoder/internal#551

We've noticed lots of flakes ingo test -race tests that use the echo provisioner. I believe the root cause of this to be#17012, where we started mutating theecho.Responses. This only caused issues as we previously sharedecho.Responses across multiple test cases.

This PR is therefore the same as#17128, but I believe this is all the cases where anecho.Responses is shared between tests - including tests that haven't flaked (yet).

@ethanndicksonGraphite App
Copy link
MemberAuthor

This stack of pull requests is managed byGraphite. Learn more aboutstacking.

@ethanndicksonethanndickson marked this pull request as ready for reviewApril 2, 2025 01:47
@ethanndicksonethanndickson changed the titlefix: avoid sharing echo.Responses across testsfix: avoid sharingecho.Responses across testsApr 2, 2025
Copy link
Contributor

@CopilotCopilotAI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors test initialization for echo.Responses to eliminate shared mutable state between tests, addressing flakiness related to the echo provisioner.

  • Convert echo.Responses variables into factory functions that return new instances.
  • Update test cases to invoke these factories to avoid data sharing across test runs.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

FileDescription
cli/update_test.goWrap echo.Responses initialization in a function and update its usages.
cli/start_test.goReplace direct echo.Responses assignment with a factory function invocation.
cli/restart_test.goApply factory function pattern to create new echo.Responses instances.
Comments suppressed due to low confidence (4)

cli/update_test.go:104

  • [nitpick] Consider renaming 'echoResponses' to 'newEchoResponses' to clarify that it returns a fresh instance on each invocation.
echoResponses := func() *echo.Responses {

cli/start_test.go:82

  • [nitpick] Consider renaming 'echoResponses' to 'newEchoResponses' for clarity since it now functions as a factory method.
echoResponses := func() *echo.Responses {

cli/restart_test.go:23

  • [nitpick] Consider renaming 'echoResponses' to 'newEchoResponses' to better indicate that it creates a new instance each time it is called.
echoResponses := func() *echo.Responses {

cli/restart_test.go:285

  • [nitpick] Consider renaming 'echoResponses' to 'newEchoResponses' to clarify its role as a factory function for generating new instances.
echoResponses := func() *echo.Responses {

@ethanndickson
Copy link
MemberAuthor

ethanndickson commentedApr 2, 2025
edited
Loading

$ go test -race -count=50 -run "TestUpdateWithRichParameters" github.com/coder/coder/v2/cliok      github.com/coder/coder/v2/cli   58.317s$ go test -race -count=50 -run "TestStart" github.com/coder/coder/v2/cliok      github.com/coder/coder/v2/cli   133.624s

Copy link
Collaborator

@sreyasreya left a comment
edited
Loading

Choose a reason for hiding this comment

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

Many blessings upon your house 🙏

@ethanndicksonethanndickson merged commit6fdad02 intomainApr 2, 2025
39 checks passed
@ethanndicksonethanndickson deleted the ethan/fix-echo-flakes branchApril 2, 2025 02:06
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsApr 2, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

Copilot code reviewCopilotCopilot left review comments

@sreyasreyasreya approved these changes

Assignees

@ethanndicksonethanndickson

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

flake: TestUpdateValidateRichParameters
2 participants
@ethanndickson@sreya

[8]ページ先頭

©2009-2025 Movatter.jp