Re: [BuildStream] Guidance on unit tests

[Tristan Van Berkom <tristan vanberkom codethink co uk>]

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.

Cheers,
    -Tristan