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

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

Hi Chiara,

On Thu, 2018-11-15 at 10:52 +0000, Chiara Tolentino via BuildStream-list wrote:
> It seems that most of the functionality needed to add testing support
> into BuildStream is already in place. I'd like to attempt to then
> summarise the remaining features needed to start supporting testing
> within BuildStream. The good news is that none of the following should
> be breaking changes!
>
> As far as I'm aware, the following items would need to be completed
> (which I'll elaborate on below):
> 1.) Add 'provides-conditions' and 'requires-conditions' fields to the
> bst element file
> 2.) Create a new kind of dependency ('abstract dependencies')
> 3.) Add functionality to 'bst show' to check the 'conditions' field(s)
> of a bst element
> 4.) Have some method of allowing a condition-providing element to use
> another element's build tree

Good spot, interestingly however, this complicates the notion that it
is in any way acceptable for an artifact to exist without it's build
tree.

> 1.) Add 'provides-conditions' and 'requires-conditions' fields to the
> bst element file
> =======================================================================================
> For the following dependency tree:
>
>       [base.bst]
>           |
>           |
>           v
>       [lib.bst] --> [lib-test.bst]
>           |
>           |
>           v
>       [app.bst] --> [app-test.bst]
>           |
>           |
>           v
>     [app-pkg.bst]
>
> Similar to what Tristan had suggested [1], if app-pkg.bst were to
> require that app.bst is tested, I'd imagine the bst files of app,
> app-test and app-pkg to look something like:
>
> ==== app.bst =====
>   kind: import
>   sources:
>   - kind: git
>     url: github: foo/app
>     track: master
>   depends:
>   - lib.bst
>   provides-conditions:
>     tested:
>     - app-test.bst
>     packaged:
>     - app-pkg.bst
>
> ==== app-test.bst ====
>   kind: manual
>   depends:
>   - app.bst
>   build-commands:
>   - setup.py test
>
> ==== app-pkg.bst ====
>   kind: manual
>   depends:
>   - app.bst
>   requires-conditions:
>   - tested
>   build-commands:
>   - setup.py sdist

Nitpick on the semantics here.

I think that the YAML would be a bit nicer if we had 'conditions' as a
key at the root level of the element YAML description.

And that 'conditions' would be a dictionary which had 'requires' and
'provides' keys in it.

Like:

  conditions:
    provides:
      tested:
      - app-test.bst

And:

  conditions:
    requires:
    - tested

Thoughts ?

> 2.) Create a new kind of dependency ('abstract dependencies')
> =============================================================
> Abstract dependencies would have the following characteristics:
> - they do not care about the output of the element that is
> 'abstractly' depended on

Right, when an 'abstract dependency' is added, it is not automatically
brought in by Element.stage_dependency_artifacts().

However there is probably a new feature of Element.dependencies() which
allows optional iteration over the abstract deps, this would be used to
construct the cache key and build plan.

> - they function like 'normal' dependencies with regards to cache key calculation
> - they are implied when an element specifies 'requires-conditions' on
> its dependencies (and so do not need to be explicitly stated in the
> bst files). To illustrate, when app-pkg.bst specifies that app.bst
> must be tested, the following abstract dependencies are created:
>
>                      [base.bst]
>                          |
>                          |
>                          v
>   [lib-test.bst] <-- [lib.bst]
>          "               |
>          "               |
>          "               v
>          "           [app.bst] --> [app-test.bst]
>          "               |               "
>          "               |               "
>          "               v               "
>          += = => [app-pkg.bst] <= = = = =+
>
> where -----> are 'normal' dependencies
> = = => are 'abstract' dependencies
>
> (The above figure is taken from [2] but modified to distinguish
> between 'normal' and 'abstract' dependencies)
>
>
> 3.) Add functionality to 'bst show' to check the 'conditions' field(s)
> of a bst element
> =======================================================================================
> For the purposes of enforcing policy (without having to rely on
> directly yaml parsing bst files), there must be a way to allow someone
> to check if particular conditions (e.g. 'tested') have been specified
> in an element's 'provides-conditions' field. Perhaps adding some
> functionality to 'bst show' to support something like:
>
>      bst show --world --format 'Name: %{name}\nProvides-conditions:
> %{provides-conditions}'
>
> which would then print out something like:
>
>   > Name: base.bst
>      Provides-conditions:
>        None
>
>      Name: lib.bst
>      Provides-conditions:
>        tested: lib-test.bst
>
>      Name: app.bst
>      Provides-conditions:
>        tested: app-test.bst
>        packaged: app-pkg.bst
>
> or perhaps be able to print out yaml like so:
>
>   > - Name: base.bst
>        Provides-conditions: None
>
>      - Name: lib.bst
>        Provides-conditions:
>          tested:
>          - lib-test.bst
>
>      - Name: app.bst
>        Provides-conditions:
>          tested:
>          - app-test.bst
>          packaged:
>          - app-pkg.bst
>
> This function would have to fail gracefully if it cannot find the
> field specified in the format string. Similarly, it might be useful to
> check if certain elements specify a 'requires-conditions' field:
>
>      bst show app-pkg.bst --deps none --format 'Name:
> %{name}\nRequires-conditions: %{requires-conditions}'
>
> which would then print out something like:
>
>   > Name: app-pkg.bst
>      Requires-conditions: tested


Interestingly, this also raises some questions around what the meaning
of user facing `--deps` arguments mean in the frontend.

When running `bst show`, are the 'abstract' dependencies considered ?

Is there a way to distinguish why the dependency was brought into the
output of `bst show` ?

Similarly we need to think out what it means for `bst fetch`, `bst
pull`, `bst push`, etc.

> 4.) Have some method of allowing a condition-providing element to use
> another element's build tree
> ==================================================================================================
> Currently, the below configuration would not work since lib-test.bst
> would only have the binary from lib.bst available to it and not the
> whole build tree. For this to then work, some method must be
> implemented to allow lib-test.bst to be built on top of lib.bst
>
> ======= lib.bst ========
> build-commands:
> - make install
> ===== lib-test.bst =====
> build-commands:
> - make test
>
> Some possible solutions discussed with Will Salmon:
> - introducing a new kind of build element which might use the previous
> element's build tree
> - use an existing element (the filter element seems like a good
> candidate but I'm not too familiar with its functionality) -- this
> could likely have big impacts on the cache
> - rebuild lib in lib-test (although this would likely slow down the
> build of app-pkg by a significant amount)

Right so the first part of it is to expose a python API for staging the
 build tree.

But as I mentioned at the beginning of this mail, the more important
question to answer right now is how to guarantee that the build tree we
want to stage in order to run tests on, actually exists in the
dependency artifact.

Currently build trees are guarantees to exist in an artifact, but this
will no longer be the case after tackling:

    https://gitlab.com/BuildStream/buildstream/issues/566

Seems we are still missing an important part of the puzzle...

Cheers,
    -Tristan