Re: Proposal: Add support for running tests in BuildStream

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

Hi Chandan,

On Tue, 2018-07-17 at 23:00 -0400, Chandan Singh wrote:
> Hi,
>
> This was discussed at GUADEC this year, and perhaps also in other settings in
> the past. But we did not have a mailing list thread or an issue about it so I
> thought I could get the ball rolling.
>
> Overview
> ========
>
> BuildStream currently does not recognize tests as being special which leads
> to some awkward setups. If one wants to run tests as part of BuildStream builds
> they generally have to choose one of the following two options:
>
> 1. Run the tests as part of the build commands, or
> 2. Have a separate element that just runs tests
>
> None of these two approaches are ideal - first approach increases the build
> times and one has to include the test dependencies as build dependencies,
> whereas the second approach doubles the number of elements as well as has the
> side effect that the build for the main element doesn't fail when the tests
> fail.
>
> Proposal
> ========
>
> One solution to this issue would be to have BuildStream recognize and handle
> tests different than builds. Tests in BuildStream should work something like
> this:
>
> - A separate test job for tests is added to the pipeline that should start
>   only after the build is finish. This job should not block other build
>   jobs from starting but should be able to mark the original build as failed
>   if the tests fail.
>
> - It would also make sense to not push any artifacts until all the tests have
>   passed.
>
> - As tests may have extra dependencies, there should be some way of declaring
>   dependencies that are only needed for running the tests.
>
> Implementation
> ~~~~~~~~~~~~~~
>
> I have not thought of the complete details yet but here's a rough sketch of
> the implementation plan:
>
> - Add a new type of dependency other than 'build' and 'run' called 'test',
>   which will be staged only while running tests.
>
> - Allow elements to define 'test-commands', which will define how to run the
>   tests.
>
> - When constructing the pipeline, queue jobs for running tests if
>   'test-commands' are defined.
>
> - Optionally, provide a way skip running the tests when doing a build.
>
>
> What do people think? Although we still need to finalize the details of how
> this would work, are there any other concerns with the high-level plan?

> From what I understand, your proposal explicitly tries to reduce the
number of elements in your pipeline - while conversely, the design of
BuildStream intends that elements remain as abstract and simple as
possible, and encourages flexibility in how projects can construct
pipelines of elements, hopefully achieving goals which we never even
anticipated.

Since I feel that this is not the first proposal which runs counter to
the core design of BuildStream by seeking to teach the base classes
more specifics which could instead be handled by companion elements, I
think there is a pattern emerging which we should be addressing instead
(or we should address first), i.e. the pattern of aversion of elements.

I will posit that in fact, while you *think* that you want to avoid
more elements in your pipeline, this is not really the case. I very
strongly suspect that what you want to avoid is maintaining more .bst
files, and you want to group logically related parts of your project
together such that extremely large projects are more easily navigable
and maintainable. There is a subtle but very interesting distinction
between these two desires.

There are a few ideas I have which I think can lead to some common
ground, and they all basically revolve around allowing the user to
express elements in their project more conveniently.

Some ideas off the top of my head which could help:

  o We are currently in the process of adding the "(@)" include
    directive, which I think could plausibly have a natural extension
    to a macro directive (e.g. and include with contextual
    substitutions).

  o YAML supports the expression of multiple fragments in a single
    stream (or file) using a "--" separator, it's possible that we
    could leverage this for the sake of declaring multiple objects in
    the same bst file - possibly even with a way to implicitly cause
    elements in the same .bst file to be interrelated in some way.

    If we had multiple related elements in a .bst file like this, we
    could possibly use subscript notation to address them, like
    "foo.bst[1]"

This is not a fully baked proposal but an illustration that we can be
very creative in how we support expression of elements at the YAML
parsing level, and these types of changes need not have dangerous core
design impacts on how Elements actually work.

Cheers,
    -Tristan