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: fix inconsistent definition of assertError function in maps section #765

Merged
merged 1 commit into from May 13, 2024

Conversation

dario-piotrowicz
Copy link
Contributor

The assertError in the maps section is introduced here:

learn-go-with-tests/maps.md

Lines 222 to 228 in 7c5b39c

func assertError(t testing.TB, got, want error) {
t.Helper()
if got != want {
t.Errorf("got error %q want %q", got, want)
}
}

as you can see with the failure error being got error "X" want "Y"

This does not match the v1 go code:

t.Errorf("got %q want %q", got, want)

Further down the section the assertError helper is refactored:

learn-go-with-tests/maps.md

Lines 367 to 375 in 7c5b39c

func assertError(t testing.TB, got, want error) {
t.Helper()
if got != want {
t.Errorf("got %q want %q", got, want)
}
}
```
For this test, we modified `Add` to return an error, which we are validating against a new error variable, `ErrWordExists`. We also modified the previous test to check for a `nil` error, as well as the `assertError` function.

to update the failure error to got "X" want "Y" (without error)

This does not really make much sense and is also not reflected anywhere in the section's go code (see: v2, v3, ..., v7)

So this PR is amending the erroneous v1 go code and removing the unnecessary/incorrect refactoring

@quii quii merged commit 9f0d3e7 into quii:main May 13, 2024
1 check passed
@dario-piotrowicz dario-piotrowicz deleted the avoid-assert-error-modification branch May 13, 2024 11:36
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.

None yet

2 participants