Code review comment for lp:~themue/juju-core/057-tailer

LGTM with the tests fixed, thanks!

https://codereview.appspot.com/36540043/diff/80001/utils/tailer/tailer_test.go
File utils/tailer/tailer_test.go (right):

https://codereview.appspot.com/36540043/diff/80001/utils/tailer/tailer_test.go#newcode42
utils/tailer/tailer_test.go:42: data: data[26:29],
On 2013/12/13 11:23:12, mue wrote:
> On 2013/12/12 14:46:39, rog wrote:
> > I like the new tests much better now. One thought though:
> >
> > I'm not convinced that referring to "data" in all these tests makes
them more
> > readable - I have no idea what's in data[26:29] without counting or
using grep
> > -n.
> >
> > Something like the below shows me exactly what's going
> > on without me needing to refer to multiple places,
> > and is independent of any changes else in the code.
> >
> > It would make it considerably easier for me to scan down
> > the tests to ensure that each one represents reasonable
> > behaviour.
> >
> > {
> > description: "lines are longer than buffer size",
> > data: []string{
> > "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz\n",
> > "0123456789012345678901234567890123456789012345678901\n",
> > "the quick brown fox ",
> > },
> > initialLinesWritten: 1,
> > initialLinesRequested: 1,
> > bufferSize: 5,
> > initialCollectedData: []string{
> > "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz\n",
> > },
> > appendedCollectedData: []string{
> > "0123456789012345678901234567890123456789012345678901\n",
> > },
> > },
> >

> This case the test table would blow up again and also would be
inconsistent
> (e.g. when using the standard larger group of lines). So now using
three
> different data variables with more speaking names as a compromise.
Also located
> them near to the table.

I'm with you about alphabetData (which is also nicely memorable, which
helps), but most of them do really benefit from having the data visible
in the test.

I processed my local copy to move much of the test data into the tests,
and two issues became immediately obvious, though I'd missed them when
cross-referencing, which I think is a reasonable indication that the
tests are not currently that clear.

Here's my modified version of the test. Only alphabetData remains. I'd
prefer it if we could use this form.

http://paste.ubuntu.com/6566524/

https://codereview.appspot.com/36540043/diff/100001/utils/tailer/tailer_test.go
File utils/tailer/tailer_test.go (right):

https://codereview.appspot.com/36540043/diff/100001/utils/tailer/tailer_test.go#newcode100
utils/tailer/tailer_test.go:100: description: "lines are
longer than buffer size, missing termination of last line",
How is the last line missing termination here?

https://codereview.appspot.com/36540043/diff/100001/utils/tailer/tailer_test.go#newcode117
utils/tailer/tailer_test.go:117: data:
unterminatedData[0:2],
The last line doesn't seem to be missing termination here.

https://codereview.appspot.com/36540043/