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



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


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