Re: [BuildStream] Guidance on unit tests

[Jim MacArthur <jim macarthur codethink co uk>]

On 16/11/2018 10:48, Tristan Van Berkom via BuildStream-list wrote:
> On Thu, 2018-11-15 at 18:28 +0000, Angelos Evripiotis via BuildStream-list wrote:
> > Hello list,
> >
> > I have opened an MR to add guidance on unit tests to the contributing guide:
> >
> > https://gitlab.com/BuildStream/buildstream/merge_requests/943
> > I’m posting it here too so that it gets a decent airing, it would be significant to all contributors.
> >
> > One aim of mine here is to decrease uncertainty around whether unit tests are welcome in the project.
> >
> > Another is to establish some common ground, before later proposing more pro-unit-test changes.
> >
> > Changes proposed in this merge request:
> >
> > State that exercising things via the cli is to be preferred. Perhaps it should also say the public API.
> >
> > State that unit tests are good where they are robust to unrelated changes made to internals.
> I'd like to ring in here and state that I personally view unit testing
> as a last resort, or useful for low level internal API surfaces (like
> the YAML layer for which we have these) which we want to test
> independently.
>
> There are a lot of problems surrounding unit testing of individual
> functions, the tests have a tendency of testing that the function does
> what it was written to do; which is subtly different from testing what
> the function *should* be doing, which is often times something that we
> will discover later on, once we realize that it was written to do
> something it shouldn't be doing... Having a test that passes is often
> an obstacle to the critical thinking that is needed when necessarily
> reconsidering what a function should be doing.
>
> I find that unit testing in the sense of writing tests before
> implementing functions was a great way for starting projects, e.g. when
> the whole end-to-end process didn't yet exist it was important; it was
> then also important to throw away all of that cruft and convert these
> to end to end tests.
>
> In summary, I feel that saying that end-to-end testing is "preferred"
> is not strongly worded enough, I would rather the statement discourage
> unit testing more strongly, and make it explicit that unit testing is
> to be avoided as much as at all possible - and that we should strive to
> convert any existing unit tests to end to end tests at all times.
>
> Further than this, one should ask themselves the question: If it is not
> possible to exercise this code path or branch statement by running
> BuildStream in any way; then what is the justification for this line of
> code to exist ?
>
> If a line of code can be proven to not be testable in an end to end
> test, it does not really mean that it should be unit tested, it rather
> means that the line of code has been proven to be dead code and code
> should rather be restructured to exclude the line.
I agree with all the above, particularly testing at the function level 
which I think is too low-level. However, I think it misses one value of 
unit tests, that they provide a direct indication of what has broken. 
You should be able to exercise all code paths via the user interface, 
just as one can test the strength of rivets by sailing dozens of ocean 
liners, but providing coverage isn't all of testing. Unit testing done 
right (it isn't always) will tell you more precisely what is broken than 
an end-to-end test.

Some of this could be improved by adding more assertions and proving 
more detailed exceptions in the code instead, but these can't capture 
the necessary context for a test in all cases.

Jim