Re: [BuildStream] Plugins migration and testing



Hi,

Thanks for writing this up Benjamin.

As for code reviews, I believe the same review policy should exist for the plugin repositories as for the core repository.  I would expect the initial set of committers to mirror the committers of Buildstream core.

I am not convinced that moving everything into buildstream-plugins-experimental is ideal.  We are now moving plugins that had a blessed status by virtue of being in the core to an area called "experimental".  I would have preferred buildstream-plugins-core or even just bst-plugins if we stuck to the move everything in core to a single repo as an intermediate.  There was already precedent for having multiple repos (bst-external, bst-plugins-experimental and bst-plugins-container), so one more wasn't going to hurt(?)

I'm actually unclear how bst-external and bst-plugins-experimental relate, as they seem to have overlapping content?  The docker source plugin seems to also be duplicated?

Cheers,

Sander

On Tue, Aug 20, 2019 at 3:37 PM Benjamin Schubert <contact benschubert me> wrote:
Hey Everyone,

TLDR: The plugin migration outside of BuildStream is breaking both
some plugins and BuildStream, I'm proposing to have more thorough code reviews
on those changes in addition to changes on how tests are run.

---

# Why is testing plugins with BuildStream important

This might seem redundant but I want my message to be clear:

Now that we move plugins out of BuildStream core, we get less test coverage
on BuildStream itself, since those plugins might be testing edge cases.
This could lead to BuildStream becoming more fragile, with more bugs, and
therefore make it less usable.

We also need to now ensure that widely used plugins, once out of the core, stay
functional for our users, which we had for free before. Now we need to be
cautious about them.

While we started moving plugins out of core, some errors went unnoticed, for
example, the `ostree` plugin, once moved was actually never tested against
BuildStream. Even now, _no_ external source plugin is tested against the
standardized source tests, because they are not registered correctly.

This can lead to breakages once we start moving more complex plugins out of
core, and I really want to avoid that.

# Problems added with the migration:

- Plugins are now installed by default when running BuildStream tests, which
means our tests are not standalone anymore.

- The standardized source tests where not checked to be run,
and are currently not running (they never were).

- BuildStream is now tested against a fixed version of the plugins that is
not updated frequently.


I want therefore to update how we deal with processes and testing external
plugins.


# On the process side:

I would like us to consider having more thorough code reviews and checks as
we do on BuildStream to avoid such problems, and ensure the process is visible.


# For testing:

This is my take on how to ensure that BuildStream doesn't break once we have
less plugins to test against.

## On BuildStream side:

External plugins that want to be tested as part of BuildStream CI should define a
methods `{package_name}.testutils.register_sources()` (We currently don't have
standardized elements tests).

This methods would call all the `register_repo_kind` necessary.

BuildStream would have a new test target (`-m sources`) which would run the
standardized source tests against each sources that declares an entrypoint in
the current environment.

Tests would be run on BuildStream CI in parallel, once against a fixed tag of
the plugins, once against their master version. The master version being
allowed to fail.

BuildStream tests that are not part of the standardized source tests should
_not_ be allowed to require external plugins and should be entirely standalone.

Integration tests are an exception to that and should be moved to their own
environment.

This would lead to three types of (tox) environments:
- pyXX
- pyXX-integration
- pyXX-plugins


## On the plugins side:

The plugins should run tests against both a pinned version of BuildStream and
against master to ensure consistency. The tests against the BuildStream master
branch being allowed to fail.

## Breaking changes:

When doing breaking changes (pre-2.0, after there won't be any so that is not a
problem), the plugins should be modified first and merged to their master
_while being broken_.



# When moving a plugin:

1) Rewrite all tests that are testing internals of BuildStream that depend
   on the plugin to be removed

2) Copy the plugin with its tests to bst-plugins-experimental
   At this point:
     - *No* code change must happen, in either repositories
     - Tests must be moved alongside the plugin
     - Source plugins should be registered in `register_sources()` in the
       plugins.


# Unanswered questions

Those questions are here merely for bookkeeping and I don't intend in resolving
all of them in this thread.

- How do we get standardized element tests?


I volunteer myself to start updating the infrastructure and the tests next week.
Please let me know if you see any issues with the plan.

Benjamin
_______________________________________________
buildstream-list mailing list
buildstream-list gnome org
https://mail.gnome.org/mailman/listinfo/buildstream-list


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]