tests for creating HTTPS, IMAP and custom port monitors #22
Conversation
bitfield
left a comment
bitfield
left a comment
There was a problem hiding this comment.
Nice PR, thank you! A few minor comments below.
This is a very good improvement to the project.
| got, err := client.CreateMonitor(create) | ||
| if err != nil { | ||
| t.Error(err) | ||
|
|
There was a problem hiding this comment.
Just a house style tip; I don't use blank lines within functions in this project.
|
|
||
| for _, tc := range tcs { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
Watch out, there's a subtle bug here: https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721
In fact, we just end up running the Custom port test four times, and none of the others.
| for _, tc := range tcs { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| client := New("dummy") |
There was a problem hiding this comment.
We should probably move the client setup outside the t.Run() function, shouldn't we?
Plan to look into addressing feedback during the next week. |
|
@mkrivoshein how are you getting on? Need any help? |
2 participants
Fixes #21