Conversation
mwbrooks
added
code health
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #320 +/- ##
==========================================
+ Coverage 64.59% 64.61% +0.02%
==========================================
Files 212 212
Lines 17755 17755
==========================================
+ Hits 11469 11473 +4
+ Misses 5210 5206 -4
Partials 1076 1076 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mwbrooks
changed the title
| func Test_AppendStringIfNotMember(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| tests := map[string]struct { |
There was a problem hiding this comment.
⭐ praise: Another thing I love about structured table tests is the guarantee of unique test names. While rare, it makes a failing test faster to find in past experience!
There was a problem hiding this comment.
Agreed and I believe we had a few duplicate names. While rare, it's just another "don't make me think" moment to have the compiler fail early.
| {name: "not_case_sensitive_success", listToCheck: []string{"hi", "hey"}, toFind: "Hey", isCaseSensitive: false, want: true}, | ||
| {name: "case_sensitive_success", listToCheck: []string{"hi", "Hey"}, toFind: "Hey", isCaseSensitive: true, want: true}, | ||
| {name: "case_sensitive_fail", listToCheck: []string{"hi", "hey", "hello", "apple", "pear"}, toFind: "Hey", isCaseSensitive: true, want: false}, | ||
| {name: "not_case_sensitive_fail", listToCheck: []string{"hi", "hey", "hello", "apple", "pear"}, toFind: "Peach", isCaseSensitive: false, want: false}, | ||
| {name: "not_case_sensitive_substring", listToCheck: []string{"hi", "hey hello"}, toFind: "hey", isCaseSensitive: false, want: false}, |
There was a problem hiding this comment.
⭐ praise: Thanks for separating fields onto new lines. It's much easier to review these changes!
There was a problem hiding this comment.
No problem and glad you agree on the single line. I wonder if we can configure our linter to enforce this?
|
Thanks for the quick Monday morning review @zimeg! Hope you enjoyed it over a nice cup of coffee or tea ☕ |
mwbrooks
deleted the
mwbrooks-table-test-consistent-p3-slice-to-map-simple
branch
2 participants
Summary
Related #315 and #316
This pull request updates our existing Table Tests using the Slice Pattern to the Map Pattern.
While the Slice Pattern is the chosen format in Golang documentation and many Golang projects, we prefer the Map Pattern for the following reasons:
There is wrong answer (as far as we know), but we're going with maps for readability and reliability.
Reviewers
This only updates the simple use-cases in order to keep the PR an easy read. We have 47 remaining Table Tests using the Slice Pattern that will need to be handled later. Over half should be on the low-to-medium risk.
Requirements