Skip to content

Conversation

@mrnugget
Copy link
Contributor

I don't know what changed but before this fix I could pretty reliably get TestBestWorkspaceCreator to fail by running:

go test -run TestBestWorkspaceCreator/different_user -count=10 ./internal/campaigns

It would fail with

--- FAIL: TestBestWorkspaceCreator (0.01s)
    --- FAIL: TestBestWorkspaceCreator/different_user (0.01s)
        expect.go:64: unexpected command: 1 error occurred:
                * unexpected argument at position 4: have="bar" want="foo"
            
        expect.go:108: unexpected number of commands executed: have=1 want=4
FAIL
FAIL    github.com/sourcegraph/src-cli/internal/campaigns       0.536s
FAIL

With the fix here I can't get it to fail anymore, so I'm 99.9% sure that the error was due to range over map not being stable and sometimes setting up the wrong assertions which would then fail.

@mrnugget mrnugget requested a review from a team January 21, 2021 10:47
Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

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

Nice catch! How many "whoat" moments did that take?

@mrnugget
Copy link
Contributor Author

How many "whoat" moments did that take?

I was honestly surprised with myself by how fast I could find it 😎

@mrnugget mrnugget merged commit 3106715 into main Jan 21, 2021
@mrnugget mrnugget deleted the mrn/fix-flaky-test branch January 21, 2021 11:06
@LawnGnome
Copy link
Contributor

Sorry, that's totally on me. Years of working with languages with inherently ordered map types has made me lazy. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants