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

[Chandan Singh <chandan chandansingh net>]

Hi Tristan,

Couple of clarifications before we dig deeper:

Reducing the number of .bst files was one of the motivations but not the
primary goal. The main issue I see with having tests as a separate element is
that if they fail, the build for the main element will still be reported as
successful. This is problematic because then if we want to ensure that the
tests are passing before generating a deployable artifact from that element, we
have to find the corresponding test element. As there is no definitive way of
finding that element, we will have to rely on naming conventions, which is not
ideal.

To mitigate this, what we currently do is to run the tests as part of the build
process. This has the obvious downside of making the build times worse,
especially if the tests are more complicated than simple unit tests. Trying to
make the tests run in parallel to the builds of the reverse dependencies is my
primary goal.


I'm not too attached to my implementation proposal and would be happy with any
other way of implementing it. But in the end I would like to be able to run
tests in a way that:

- the build for the element fails if the tests fail
- ideally, they don't block builds of its reverse-dependencies

If we can achieve the above with some yaml magic, I would be happy to treat
that as a solution to this problem.


Coming back to your last message.

>   o That an element now has different dependencies depending on what
>     it's doing... does this mean even more cache keys ?

Probably yes, if we go with my current proposal. I suppose it would introduce
a new form of "unsafe" cache keys. I haven't fully figured out the
implementation just yet but here's how I imagine it would work at a high-level:

- once the build finishes, an artifact a stored with unsafe cache key
- at this point, elements depending on it can use this artifact but their
  resulting artifacts will also be marked unsafe
- when trying to find a dependency in cache, elements should give less priority
  to the artifacts corresponding to the unsafe cache key
- if the tests pass, the proper cache key will also point to the same
  artifact and then it can be pushed
- if the tests fail, the artifacts are scheduled for deletion

> Another point that this ignores is that tests are not necessarily
> relevant to every kind of element, but are certainly relevant to the
> build elements.

Fair point. I agree I was mostly thinking about BuildElements. Special-casing
them would not be ideal if we can avoid it.

Very interested in hearing any suggestions on how to tackle this problem.

Cheers,
Chandan
On Thu, Jul 19, 2018 at 4:09 AM Tristan Van Berkom
<tristan vanberkom codethink co uk> wrote:
> On Thu, 2018-07-19 at 08:08 +0100, Paul Sherwood wrote:
> > On 2018-07-19 07:52, Tristan Van Berkom via Buildstream-list wrote:
> [...]
> > Now Tristan's comment...
> >
> > > From what I understand, your proposal explicitly tries to reduce the
> > > number of elements in your pipeline - while conversely, the design of
> >
> > <snip>
> >
> > While I think it's correct that Chandan has an (implied) interest in
> > avoiding too many bst files, I don't think that's the main drive for his
> > proposal.
> >
> > Regarding organisation of elements vs bst files, the approach we ended
> > up with in YBD was to separate by convention, not by enforcement in
> > code. For example YBD can parse a whole cluster (including set of
> > systems, made up of strata, with all component *commands etc) from a
> > single file. I tried to make this as permissive as possible for users,
> > by scanning the whole directory tree looking for what might count as
> > applicable definitions files.
>
> Right, the data model in YBD is significantly different in this regard
> as the core had intimate knowledge of the data types (e.g. "strata
> contain chunks" etc).
>
> That said, I think that some productivity features for the users
> convenience at this level can certainly be added to BuildStream to
> allow people to maintain their projects in ways that are more
> convenient for their particular projects.
>
> I did not go into specifics of Chandan's proposal as I expect he
> already has some understanding of how impactful the proposed changes
> are, but some points for the wider readership...
>
> Two of the high impact implications which are not explored in depth in
> Chanan's propsal are:
>
>   o That an element now has different dependencies depending on what
>     it's doing... does this mean even more cache keys ?
>
>     It's highly desirable to keep the dependency model simple
>
>   o The proposal implies that an artifact can be created and consumed
>     by other elements as dependencies... but the artifact can later be
>     modified by a test (this is what I understand by causing the "same
>     element" to fail).
>
>     One might argue that the artifact does not exist until it has been
>     pushed to a server where artifacts are stored and shared, and that
>     we mitigate the existence of failed artifacts by not sharing them,
>     but I think this is incorrect. Rather, the sharing of artifacts
>     should remain a productivity feature but not a requirement for the
>     safe operation of BuildStream.
>
> Another point that this ignores is that tests are not necessarily
> relevant to every kind of element, but are certainly relevant to the
> build elements.
>
> My argument is that there are other ways to approach this which are
> safer and can be achieved without aggressive redesign - using a
> separate element in the data model (but not necessarily in the user's
> .bst files) should allow this.
>
> Cheers,
>     -Tristan


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