Re: [BuildStream] Proposal: Add support for running tests in BuildStream

[Chandan Singh <chandan chandansingh net>]

Hi all,

After several long discussions in this thread, another thread [0], and in
person at the recent BuildStream Gathering [1], I think we have finally reached
some consensus on how testing should work in BuildStream. In this message, I
aim to summarize what we discussed in person. Unless anyone has any objections
with the high-level plan, I will summarize the implementation plan in a
following message.

Problem
=======

Let's start by doing a recap of the problem we are trying to solve.

As software integrators, we would like to run tests as part of BuildStream
builds and be able to validate (at a later stage in build) that the tests for a
given element have passed. Ideally, such tests should have the following
properties:

  1.) Tests should not block reverse dependency builds.
  2.) Strong association between an element and its tests.
  3.) Additional dependencies for testing an element should not be required
      while building it.
  4.) Elements should be able to assert that the tests for their dependencies
      have passed.
  5.) Test elements are not necessarily pass-through elements - they can have
      meaningful output such as coverage reports.

Note that having tests as separate elements can result in essentially doubling
the number of elements in a given project - this problem is orthogonal to
testing and also applies to things like packaging. So, let us acknowledge that
is important and deal with it separately.

Proposed solution
=================

To ensure that all of us are talking about the same thing, let us consider the
following dependency graph as an example.

      [base.bst]
          |
          |
          v
      [lib.bst] --> [lib-test.bst]
          |
          |
          v
      [app.bst] --> [app-test.bst]
          |
          |
          v
    [app-pkg.bst]

This is a simple project with a single app and a single library, where the
final output is a package of the app. Other than the standard elements for the
base, lib and app; we also have a few additional elements:

  a.) lib-test.bst - a standard BuildStream element that will run tests for
      lib.bst as part of its build.
  b.) app-test.bst - same as lib-test.bst but will run tests for app.bst.
  c.) app-pkg.bst - a standard BuildStream element that will produce a package
      for the app, depends only on app.bst.

If this project is used as-is, there will be no way for app-pkg.bst to verify
that the tests have actually passed. Moreover, since lib-test.bst is not listed
as a dependency for anything in the plan for app-pkg.bst, it will not even get
built. To remedy that, we need to introduce the notion of "conditions".

XXX: Maybe we need a better word for "conditions" - suggestions welcome :)

Conditions
----------

Conditions will essentially be a mapping from a keyword to a set of elements.
The keyword can be anything as far as BuildStream is concerned but the projects
are free to enforce their own guidelines. The set of elements will usually be a
subset of the reverse dependencies of the given element.

Conditions can be things like: "tested", "packaged", "license verified" etc.

When listing dependencies, an element can require certain conditions to be
true for its dependencies. Doing so will implicitly add a dependency on the
corresponding set of elements.

Coming back to our example, lib.bst can declare its "tested" condition to be
the set of elements: [lib-test.bst]. Then, app-pkg.bst can require the "tested"
condition to be met for its dependencies. So, although it only explicitly
depends on app.bst, BuildStream would automatically add a dependency on
lib-test.bst and app-test.bst when building it.

It is also worth noting that since base.bst does not have a corresponding test
element and does not declare a "tested" condition, this condition does not
apply to it.

While this example only mentions the "tested" condition, it should be easy to
see how this might work with other conditions as well.

Considerations
==============

Using this approach, we can satisfy the criteria listed above. Additionally, it
also provides the following advantages:

  - BuildStream does not require any implicit knowledge about testing and
    treats tests as regular elements.
  - Since the idea of conditions is generic, it can also be used for other
    purposes like "packaged", "license verified" etc.
  - It is easily enforceable by maintainers of a BuildStream project, either
    manually or automatically.

At various points in the past, we have discussed other approaches. Here is a
short (and incomplete) list of approaches that we considered but realized were
not fit for purpose for some reason. I am listing them here in case anyone
thinks that we missed some obvious choices:

   - Running tests as part of the build - basically works but we lose the
     potential parallelism and block the reverse dependency builds.
   - Retroactively marking build as failed if tests fail: altering the artifact
     after it has been published seems wrong, and failing a test should not
     prevent the existence of the artifact for a successful build.
   - Having leaf elements explicitly depend on the test elements - basically
     works but not easy to enforce standards this way and it is easy to miss
     tests for certain dependencies.
   - Have additional cache keys for testing - requires BuildStream to be aware
     that tests are special and also suffers from the same problem as having an
     explicit dependency on test elements.


I have purposefully tried to keep the implementation details out of this
message, so that we can agree on the high-level plan first, and decide on the
details later. I will be very curious to hear what others think about
"conditions" :)


[0]: https://mail.gnome.org/archives/buildstream-list/2018-October/msg00041.html
[1]: 
https://wiki.gnome.org/action/quicklink/Projects/BuildStream/Events?action=quicklink#BuildStream_Gathering_2018

Cheers,
Chandan

PS: Thanks to everyone who participated in the discussions. This is definitely
not something I came up with on my own.
On Thu, Aug 2, 2018 at 6:48 AM Tristan Van Berkom
<tristan vanberkom codethink co uk> wrote:
> On Wed, 2018-08-01 at 18:17 +0100, Chandan Singh wrote:
> > Hi Tristan,
> [...]
> > > The point is that:
> > >
> > > * The existence of the BuildElement's artifact is a statement that the
> > >   code has built successfully.
> > >
> > > * The existence of the testing element's artifact is a statement that
> > >   the built code has also been tested.
> > >
> > > * BuildElements which depend on other BuildElements for the purpose of
> > >   building, do not require that other testing elements have also
> > >   passed.
> > >
> > >   Note: this is what affords you the parallelism of builds not blocking
> > >         on the testing of elements you depend on.
> >
> > This is the part where I am not very convinced :) The existence of the
> > BuildElement's artifact is surely a statement that the code has been built
> > successfully. But as a user what I care about the most is not whether or not
> > it is built successfully but rather "is the output integratable?".
>
> Here is the interesting part, "is the output integratable ?"
>
> The design of BuildStream as it stands, without a massive redesign
> which I think we don't need; will satisfy this requirement.
>
> You continue to insist however that the output of your pipeline *is
> every artifact*, but that runs counter to the design.
>
> Further, even if we baked this whole thing into the base Element, if
> you were to start builds in parallel, there would be *something* that
> exists which those elements can stage and build things against whilst
> the tests are running in parallel: This thing is currently called an
> artifact, and the statement that the corresponding tests have passed
> should be another artifact.
>
> There most certainly must be a way to express a pipeline which does
> what you want, when we ask the question "is the output integratable ?",
> we should be talking about a tested artifact, not every single artifact
> in the pipeline.
>
> When building a Linux firmware for instance, there is always a stage
> after the builds of every element, before the composition stage, where
> one can depend on the tested artifacts.
>
> If understand correctly, your use case involves generating packages;
> and if I were to spell out the requirement I believe you are hinting at
> (but don't think has been adequately described in this conversation) in
> my own words, I would say that requirement is:
>
>   o I want the creation of a deployable package artifact to be
>     contingent on it's input artifact having passed tests.
>
>   o Further, I want the creation of a deployable package artifact
>     to be contingent on the input artifact's dependencies all having
>     passed tests also.
>
> Did I accurately describe the problem ?
>
> The graphs in this email do not address the requirement above, and
> perhaps there is indeed a need for a core enhancement in order to
> achieve this. This core enhancement could be something along the lines
> of describing roles and relationships between an element and other
> related elements, and these relationships could be leveraged to
> automate a transformation on a dependency tree.
>
> For instance, one might have to make the statement that:
>
>   Creating this deployable package depends on the "test" elements
>   related to this deployable package's build element, and all of
>   it's build dependencies, recursively.
>
> I.e. a way to annotate the role of the relationship (in this case
> "test"), and a way to generate a dependency tree which depends on
> elements which are related via a role, rather than the elements
> themselves, could accomplish this (think of this like depending on a
> "test" shadow of the build dependency tree, instead of the dependency
> tree itself).
>
> There must still be a few more ways to fry this cat, and seasonings we
> can apply to reduce it's chewyness (like the YAML improvements which we
> should discuss separately).
>
> The above is one plausible approach, and it is not elaborated to my
> satisfaction (it's just an idea at this stage), but I *think* you have
> enough to go on, or might come up with another approach for making this
> kind of association conveniently ?
>
> Cheers,
>     -Tristan


-- 
Regards,
Chandan Singh
http://chandansingh.net/