Skip to content

Commit fd8a537

Browse files
quiiquii
committed
wip - cl - teasing towards using bufio.NewReader
1 parent 513d275 commit fd8a537

15 files changed

+1087
-1
lines changed

Diff for: ‎.gitignore

+2
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,5 @@ _book/
88
*.epub
99
*.mobi
1010
*.pdf
11+
12+
game.db.json

Diff for: ‎command-line.md

+374-1
Original file line numberDiff line numberDiff line change
@@ -203,5 +203,378 @@ That should make it pass.
203203

204204
Next we need to simulate reading from `Stdin` (the input from the user) so that we can record wins for specific players.
205205

206-
Let's add a new test to exercise this.
206+
Let's extend our test to exercise this
207+
208+
## Write the test first
209+
210+
```go
211+
func TestCLI(t *testing.T) {
212+
in := strings.NewReader("Chris wins\n")
213+
playerStore := &StubPlayerStore{}
214+
215+
cli := &PokerCLI{playerStore, in}
216+
cli.PlayPoker()
217+
218+
if len(playerStore.winCalls) < 1 {
219+
t.Fatal("expected a win call but didnt get any")
220+
}
221+
222+
got := playerStore.winCalls[0]
223+
want := "Chris"
224+
225+
if got != want {
226+
t.Errorf("didnt record correct winner, got '%s', want '%s'", got, want)
227+
}
228+
}
229+
```
230+
231+
`os.Stdin` is what we'll use in `main` to capture the user's input. It is a `*File` under the hood which means it implements `io.Reader` which as we know by now is a handy way of capturing text.
232+
233+
We create a `io.Reader` in our test using the handy `strings.NewReader`, filling it with what we expect the user to type.
234+
235+
## Try to run the test
236+
237+
`./PokerCLI_test.go:12:32: too many values in struct initializer`
238+
239+
## Write the minimal amount of code for the test to run and check the failing test output
240+
241+
We need to add our new dependency into `PokerCLI`.
242+
243+
```go
244+
type PokerCLI struct {
245+
playerStore PlayerStore
246+
in io.Reader
247+
}
248+
```
249+
250+
## Write enough code to make it pass
251+
252+
```
253+
--- FAIL: TestCLI (0.00s)
254+
PokerCLI_test.go:23: didnt record correct winner, got 'Cleo', want 'Chris'
255+
FAIL
256+
```
257+
258+
Remember to do the strictly easiest thing first
259+
260+
```go
261+
func (cli *PokerCLI) PlayPoker() {
262+
cli.playerStore.RecordWin("Chris")
263+
}
264+
```
265+
266+
The test passes. We'll add another test to force us to write some real code next, but first let's refactor
267+
268+
## Refactor
269+
270+
In `server_test` we earlier did checks to see if wins are recorded like we have here. Let's DRY that assertion up into a helper
271+
272+
```go
273+
func assertPlayerWin(t *testing.T, store *StubPlayerStore, winner string) {
274+
t.Helper()
275+
276+
if len(store.winCalls) != 1 {
277+
t.Fatalf("got %d calls to RecordWin want %d", len(store.winCalls), 1)
278+
}
279+
280+
if store.winCalls[0] != winner {
281+
t.Errorf("did not store correct winner got '%s' want '%s'", store.winCalls[0], winner)
282+
}
283+
}
284+
```
285+
286+
Now replace the assertions in both `server_test.go` and `PokerCLI_test.go`
287+
288+
The test should now read like so
289+
290+
```go
291+
func TestCLI(t *testing.T) {
292+
in := strings.NewReader("Chris wins\n")
293+
playerStore := &StubPlayerStore{}
294+
295+
cli := &PokerCLI{playerStore, in}
296+
cli.PlayPoker()
297+
298+
assertPlayerWin(t, playerStore, "Chris")
299+
}
300+
```
301+
302+
Now let's write _another_ test with different user input to force us into actually reading it.
303+
304+
## Write the test first
305+
306+
```go
307+
func TestCLI(t *testing.T) {
308+
309+
t.Run("record chris win from user input", func(t *testing.T) {
310+
in := strings.NewReader("Chris wins\n")
311+
playerStore := &StubPlayerStore{}
312+
313+
cli := &PokerCLI{playerStore, in}
314+
cli.PlayPoker()
315+
316+
assertPlayerWin(t, playerStore, "Chris")
317+
})
318+
319+
t.Run("record cleo win from user input", func(t *testing.T) {
320+
in := strings.NewReader("Cleo wins\n")
321+
playerStore := &StubPlayerStore{}
322+
323+
cli := &PokerCLI{playerStore, in}
324+
cli.PlayPoker()
325+
326+
assertPlayerWin(t, playerStore, "Cleo")
327+
})
328+
329+
}
330+
```
331+
332+
## Try to run the test
333+
334+
```
335+
=== RUN TestCLI
336+
--- FAIL: TestCLI (0.00s)
337+
=== RUN TestCLI/record_chris_win_from_user_input
338+
--- PASS: TestCLI/record_chris_win_from_user_input (0.00s)
339+
=== RUN TestCLI/record_cleo_win_from_user_input
340+
--- FAIL: TestCLI/record_cleo_win_from_user_input (0.00s)
341+
PokerCLI_test.go:27: did not store correct winner got 'Chris' want 'Cleo'
342+
FAIL
343+
```
344+
345+
## Write enough code to make it pass
346+
347+
```go
348+
func (cli *PokerCLI) PlayPoker() {
349+
userInput, _ := ioutil.ReadAll(cli.in)
350+
351+
winner := strings.Replace(string(userInput), " wins\n", "", -1)
352+
cli.playerStore.RecordWin(winner)
353+
}
354+
```
355+
356+
The easiest way to make this test pass is:
357+
- Read all the contents of the string
358+
- Extract out the winner by using `strings.Replace` which takes the string to replace, what string to replace, its replacement and finally a flag to say how many instances to replace
359+
360+
## Refactor
361+
362+
We can extract getting the winner's name into a meaningful function
363+
364+
```go
365+
func (cli *PokerCLI) PlayPoker() {
366+
userInput, _ := ioutil.ReadAll(cli.in)
367+
368+
cli.playerStore.RecordWin(extractWinner(userInput))
369+
}
370+
371+
func extractWinner(userInput []byte) string {
372+
return strings.Replace(string(userInput), " wins\n", "", 1)
373+
}
374+
```
375+
376+
Now that we have some working(ish) software, we should wire this up into `main`. Remember we should always strive to have fully-integrated working software as quickly as we can.
377+
378+
In `main.go` add the following and run it.
379+
380+
```go
381+
package main
382+
383+
import (
384+
"fmt"
385+
"github.com/quii/learn-go-with-tests/command-line/v3"
386+
"log"
387+
"os"
388+
)
389+
390+
const dbFileName = "game.db.json"
391+
392+
func main() {
393+
fmt.Println("Let's play poker")
394+
fmt.Println("Type {Name} wins to record a win")
395+
396+
db, err := os.OpenFile(dbFileName, os.O_RDWR|os.O_CREATE, 0666)
397+
398+
if err != nil {
399+
log.Fatalf("problem opening %s %v", dbFileName, err)
400+
}
401+
402+
store, err := poker.NewFileSystemPlayerStore(db)
403+
404+
if err != nil {
405+
log.Fatalf("problem creating file system player store, %v ", err)
406+
}
407+
408+
if err != nil{
409+
log.Fatalf("problem creating ")
410+
}
411+
412+
game := poker.PokerCLI{store, os.Stdin}
413+
game.PlayPoker()
414+
}
415+
```
416+
417+
You should get an error
418+
419+
```
420+
command-line/v3/cmd/cli/main.go:32:25: implicit assignment of unexported field 'playerStore' in poker.PokerCLI literal
421+
command-line/v3/cmd/cli/main.go:32:34: implicit assignment of unexported field 'in' in poker.PokerCLI literal
422+
```
423+
424+
This highlights the importance of _integrating your work_. We rightfully made the dependencies of our `PokerCLI` private but haven't made a way for users to construct it.
425+
426+
Is there a way to have caught this problem earlier?
427+
428+
### `package mypackage_test`
429+
430+
In all other examples so far when we make a test file we declare it as being in the same package that we are testing.
431+
432+
This is fine and it means on the odd occasion where we want to test something internal to the package we have access to the unexported types.
433+
434+
But given we have advocated for _not_ testing internal things _generally_, can Go help enforce that? What if we could test our code where we only have access to the exported types (like our `main` does)?
435+
436+
When you're writing a project with different packages I would strongly recommend that your test package name has `_test` at the end. When you do this you will only be able to have access to the public types in your package. This would help with this specific case but also helps enforce the discipline of only testing public APIs.
437+
438+
An adage with TDD is that if you cannot test your code then it is probably poor code to work with. Using `package foo_test` will help with this.
439+
440+
Before fixing `main` let's change the package of our test inside `PokerCLI_test` to `poker_test`
441+
442+
If you have a well configured IDE you will suddenly see a lot of red! If you run the compiler you'll get the following errors
443+
444+
```
445+
./PokerCLI_test.go:12:19: undefined: StubPlayerStore
446+
./PokerCLI_test.go:14:11: undefined: PokerCLI
447+
./PokerCLI_test.go:17:3: undefined: assertPlayerWin
448+
./PokerCLI_test.go:22:19: undefined: StubPlayerStore
449+
./PokerCLI_test.go:24:11: undefined: PokerCLI
450+
./PokerCLI_test.go:27:3: undefined: assertPlayerWin
451+
```
452+
453+
We have now stumbled into more questions on package design.
454+
455+
#### `Do we want to have our stubs and helpers 'public' ?`
456+
457+
This is a subjective discussion. One could definitely argue that you do not want to pollute your package's API with code to facilitate tests.
458+
459+
In the presentation ["Advanced Testing with Go"](https://speakerdeck.com/mitchellh/advanced-testing-with-go?slide=53) by Mitchell Hashimoto it is described how at HashiCorp they advocate doing this so that users of the package can write tests without having to re-invent the wheel writing stubs. In our case this would mean anyone using our poker package wont have to create their own stub `PlayerStore` if they wish to work with our code.
460+
461+
Anecdotally I have used this technique in other shared packages and it has proved extremely useful in terms of users saving time when integrating with our packages.
462+
463+
So let's create a file called `testing.go` and add our stub and our helpers.
464+
465+
```go
466+
package poker
467+
468+
import "testing"
469+
470+
type StubPlayerStore struct {
471+
scores map[string]int
472+
winCalls []string
473+
league []Player
474+
}
475+
476+
func (s *StubPlayerStore) GetPlayerScore(name string) int {
477+
score := s.scores[name]
478+
return score
479+
}
480+
481+
func (s *StubPlayerStore) RecordWin(name string) {
482+
s.winCalls = append(s.winCalls, name)
483+
}
484+
485+
func (s *StubPlayerStore) GetLeague() League {
486+
return s.league
487+
}
488+
489+
func AssertPlayerWin(t *testing.T, store *StubPlayerStore, winner string) {
490+
t.Helper()
491+
492+
if len(store.winCalls) != 1 {
493+
t.Fatalf("got %d calls to RecordWin want %d", len(store.winCalls), 1)
494+
}
495+
496+
if store.winCalls[0] != winner {
497+
t.Errorf("did not store correct winner got '%s' want '%s'", store.winCalls[0], winner)
498+
}
499+
}
500+
501+
// todo for you - the rest of the helpers
502+
```
503+
504+
You'll need to make the helpers public (remember exporting is done with a capital letter at the start) if you want them to be exposed to importers of our package.
505+
506+
In our CLI test you'll need to call the code as if you were using it within a different package.
507+
508+
```go
509+
func TestCLI(t *testing.T) {
510+
511+
t.Run("record chris win from user input", func(t *testing.T) {
512+
in := strings.NewReader("Chris wins\n")
513+
playerStore := &poker.StubPlayerStore{}
514+
515+
cli := &poker.PokerCLI{playerStore, in}
516+
cli.PlayPoker()
517+
518+
poker.AssertPlayerWin(t, playerStore, "Chris")
519+
})
520+
521+
t.Run("record cleo win from user input", func(t *testing.T) {
522+
in := strings.NewReader("Cleo wins\n")
523+
playerStore := &poker.StubPlayerStore{}
524+
525+
cli := &poker.PokerCLI{playerStore, in}
526+
cli.PlayPoker()
527+
528+
poker.AssertPlayerWin(t, playerStore, "Cleo")
529+
})
530+
531+
}
532+
```
533+
534+
You'll now see we have the same problems as we had in `main`
535+
536+
```
537+
./PokerCLI_test.go:15:26: implicit assignment of unexported field 'playerStore' in poker.PokerCLI literal
538+
./PokerCLI_test.go:15:39: implicit assignment of unexported field 'in' in poker.PokerCLI literal
539+
./PokerCLI_test.go:25:26: implicit assignment of unexported field 'playerStore' in poker.PokerCLI literal
540+
./PokerCLI_test.go:25:39: implicit assignment of unexported field 'in' in poker.PokerCLI literal
541+
```
542+
543+
The easiest way to get around this is to make a constructor as we have for other types
544+
545+
```go
546+
func NewPokerCLI(store PlayerStore, in io.Reader) *PokerCLI {
547+
return &PokerCLI{
548+
playerStore:store,
549+
in:in,
550+
}
551+
}
552+
```
553+
554+
Change the test to use the constructor instead and we should be back to the tests passing
555+
556+
Finally, we can go back to our new `main.go` and use the constructor we just made
557+
558+
```go
559+
game := poker.NewPokerCLI(store, os.Stdin)
560+
```
561+
562+
Try and run it, type "Bob wins".
563+
564+
### You cannot read "all" of os.Stdin
565+
566+
Nothing happens! You'll have to force the process to quit. What's going on?
567+
568+
As an experiment change the code to the following
569+
570+
```go
571+
func (cli *PokerCLI) PlayPoker() {
572+
log.Println("1")
573+
userInput, _ := ioutil.ReadAll(cli.in)
574+
log.Println("2")
575+
cli.playerStore.RecordWin(extractWinner(userInput))
576+
}
577+
```
578+
579+
No matter what you type, you never see `2` logged. The reason is the `ReadAll`, we cant read "all" of `os.Stdin`, as you can just keep typing stuff in!
207580

0 commit comments

Comments
 (0)