Implement letters for lab 03 of go-course #513
Conversation
Shortening variable names Implements test of sortLetters Implement extensive test cases Add main_test to git tracking
undrewb
requested review from
anzboi,
anzdaddy,
camscale,
joshcarp,
juliaogris,
linuxmonk,
pentaphobe and
rokane
as code owners
Codecov Report
@@ Coverage Diff @@
## master #513 +/- ##
=======================================
Coverage 100% 100%
=======================================
Files 73 171 +98
Lines 1273 3184 +1911
=======================================
+ Hits 1273 3184 +1911
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Looks pretty solid - just please get rid of the unnecessary comments as @runnerdave suggested.
| input string | ||
| want map[rune]int | ||
| }{ | ||
| {name: "lab example", input: "aba", want: map[rune]int{97: 2, 98: 1}}, |
There was a problem hiding this comment.
Can you please update the values for the want section of each case so you are using the actual char values rather than int values for the runes? Makes the test cases a lot easier to read.
{name: "lab example", input: "aba", want: map[rune]int{'a': 2, 'b': 1}},|
|
||
| func TestLetters(t *testing.T) { | ||
| for _, tt := range lettersTestData { | ||
| tt := tt // as per sean- suggestion on this discussion https://github.com/kyoh86/scopelint/issues/4 |
There was a problem hiding this comment.
I think this might have been suggested already, but I don't think this comment is required. Can you please remove this?
| }{ | ||
| {name: "lab example", input: "aba", want: "a:2,b:1"}, | ||
| {name: "trailing newline", input: "aba\n", want: "\n:1,a:2,b:1"}, | ||
| {name: "duplicate entries", input: "\t\t\\aba\n", want: "\t:2,\n:1,\\:1,a:2,b:1"}, |
There was a problem hiding this comment.
I understand that you are trying to achieve good test coverage, but I think this might be a little bit of overkill. It's important to consider what each test is trying to achieve when building out test cases. I feel that a lot of these cases are kind of testing the same thing (one repeated letter, random example, whitespace, duplicate entries, lab example) and many of these can be removed.
If you think about structuring your tests and having each test try to achieve / verify something in particular, you should be able to hit most of these cases with a couple of tests.
- Empty input -> Corner case
- Multiple letters (different languages if you want) -> Positive Case
- Emoji input -> Verify data structure is using rune
|
The go-course is now closed. Thank you very much for participating. |
Shortening variable names
Implements test of sortLetters
Implement extensive test cases
Fixes #512
Review of colleague's PR #509
Implement lab3 of anz-bank go-course