Re: [BuildStream] Please try tox ! [Was: Update CI and testing process]

[Angelos Evripiotis <angelos evripiotis gmail com>]

On Mon, 14 Jan 2019 at 21:42, Chandan Singh <chandan chandansingh net> wrote:

Hi Angelos, Tristan,

On Mon, Jan 14, 2019 at 3:03 PM Tristan Van Berkom
<tristan vanberkom codethink co uk> wrote:

<snip>

> > I don't know enough about Python packaging to make a recommendation
> > about '--develop' or 'skipsdist=True'. Maybe I'll come back to that
> > another time, I find it saves a few seconds for our project:
> > https://tox.readthedocs.io/en/latest/example/general.html#avoiding-expensive-sdist
>
> Well, we removed a lot of cruft from `.gitlab-ci.yml` expressely
> *because* tox takes cares of creating and installing a package for us,
> i.e. the intention is to run all tests from an installed package and
> that it ensures at all times that the package has everything it needs.
>
> We could probably make it not install a package by default and only do
> it in CI... but I think it will be a challenge to ensure that coverage
> information is collected properly in every scenario.

I agree with Tristan here that at least the CI should continue to use
`tox` without these options. Always doing a sdist install helps us
catch any issues related to packaging of BuildStream. To illustrate
this, I have created a `chandan/tox-demo-skip-sdist` branch [1].

In this branch, I essentially removed a bunch of CI jobs that weren't
relevant to this experiment, and added two new jobs: `normal-test` and
`skip-sdist`. Both jobs remove the MANIFEST.in file before the build,
which means that our package will be missing a bunch of files that we
expect to have. Both jobs do identical stuff, except that skip-sdist
job provides an additional `--develop` option to tox.

As you will see from the pipelines [2] and [3], normal-test job fails,
as expected. Whereas the skip-sdist job happily passes. So, if we were
to run with this setup in the CI, we will fail to catch issues where
we do not have correct metadata for properly creating our package.

Doing it locally may make sense if people know what they are doing.
Otherwise, we will end up with the classic problem of "works on my
laptop, fails on CI".

Cheers,
Chandan

[1]: https://gitlab.com/BuildStream/buildstream/commits/chandan/tox-demo-skip-sdist
[2]: https://gitlab.com/BuildStream/buildstream/-/jobs/145682835
[3]: https://gitlab.com/BuildStream/buildstream/-/jobs/145682836

Wow, thanks for this - it makes it clear   ‘skipsdist’ isn’t for us in the general case, and why.

I’ll see how my mileage goes in local use, so we can judge if it’s worth any confusion that comes with mentioning ‘—develop’ in contributing.rst.

Also worth noting that switching back and forth between ‘—develop’ and not causes a rebuild of the environment- pretty expensive!

Cheers!

Angelos