miyuchina / mistletoe

A fast, extensible and spec-compliant Markdown parser in pure Python.
MIT License
841 stars 119 forks source link

Make CI work again #121

Closed pbodnar closed 1 year ago

pbodnar commented 3 years ago

The build on Travis seems to be broken for 2 main reasons now:

  1. Python 3.3 seems to be no longer available for download and builds, that's why the last builds kept failing. This version should be therefore possibly removed from ".travis.yml".
  2. Builds no longer run on travis-ci.org - migration to travis-ci.com needs to be done (short setup tutorial). While builds seem to run there nicely (just testing it now on my forked repo), it looks like they are not completely free and require an extra support request (of an unspecified form) after trial credits are over (see this post; Travis UI doesn't give too much hints on this so far).

An alternative approach is to use something else than Travis, like GitHub Actions, but I'm not quite familiar with those yet.

pbodnar commented 2 years ago

FYI, the integration with Travis CI needs to be setup in "Settings > Integrations > Integrated apps" - only project / account maintainers have got access there though. If we decide to go this way, we will have to ask @miyuchina then...

zyv commented 2 years ago

Does it even make sense? Maybe just migrate to built-in GitHub Actions instead?

pbodnar commented 2 years ago

Does it even make sense? Maybe just migrate to built-in GitHub Actions instead?

Sure, GitHub Actions might be also the way to go, as I write:

An alternative approach is to use something else than Travis, like GitHub Actions, but I'm not quite familiar with those yet.

Tips are welcome. :)

pbodnar commented 1 year ago

Getting closer: https://github.com/pbodnar/mistletoe/actions/runs/4107835671 :) The migration to GitHub seems really trivial, just some extra cases (like missing Python 3.6- in the latest Ubuntu) need to be covered. I've also fixed some errors in the legacy code, so that the basic build (linter) won't fail. I guess I will soon push a complete workflow to this repo...

zyv commented 1 year ago

Getting closer: https://github.com/pbodnar/mistletoe/actions/runs/4107835671 :) The migration to GitHub seems really trivial, just some extra cases (like missing Python 3.6- in the latest Ubuntu) need to be covered. I've also fixed some errors in the legacy code, so that the basic build (linter) won't fail. I guess I will soon push a complete workflow to this repo...

Just a tip: you are using an outdated version of setup-python - v3 and the current one is v4. This why you are getting deprecation warnings about set-output and your runs will start failing soon.

To keep on top of the actions you can enable Dependabot by creating .github/dependabot.yml along those lines:

version: 2
updates:

   - package-ecosystem: "github-actions"
     directory: "/"
     commit-message:
       prefix: "chore(ci): "
     schedule:
       interval: "weekly"
     open-pull-requests-limit: 1
zyv commented 1 year ago

Also note that according to the Travis configuration, there is some custom-made weird harness running spec tests. This has to be added to the workflow, or else made compatible with pytest.

Also you need a plugin for measuring coverage via pytest. Be sure to exclude the tests themselves from coverage though, this is a classic pitfall when migrating from plain coverage reporter and unittest harness. I use the following:

poetry run pytest --cov --cov-fail-under=90
[tool.coverage.run]
branch = true
source = ["."]
omit = [
    "*/migrations/*",
    "*/tests/*",
    "*/test_*.py",
    "*/tests.py",
    "manage.py"
]
pbodnar commented 1 year ago

@zyv, thanks a lot for the tips, I've already implemented the 1st one and I'm going to gradually get to the rest as well.

pbodnar commented 1 year ago

OK, so I hope I have successfully finished the migration from Travis to GitHub Actions. 🎉 Let's see how it works for us in practice and tune it as needed. Of course, further remarks or tips are welcome. :) (There are so many ways how to setup these things!)

Here are my remarks from the migration:

  1. The build, tests, coverage report, and newly also checking code problems by Flake8 linter (taken from the GitHub's workflow template), are all controlled by the workflow at https://github.com/miyuchina/mistletoe/actions/workflows/python-package.yml. This replaces the old .travis.yml.

  2. I can't see / administer the current coverage threshold on Coveralls, as I'm not the owner of this repo. If this leads to refusing PRs because of code coverage, we would need to either ask @miyuchina for a change or administration rights, or give up on Coveralls and control the threshold just in our GitHub workflow (e.g. coverage report --fail-under=90).

  3. Previously, lines of code from the "test" folder were part of the coverage stats - this is no longer true thanks to using the --source switch (as hinted by @zyv - other switches could be also used if more control is needed).

  4. Without the "pyproject.toml" project file to keep it simple for now. It would contain probably only something like this, just for the coverage tool:

    Configuring the "coverage" tool below - see https://coverage.readthedocs.io/en/latest/config.html

    [tool.coverage.run] source = [ "mistletoe", ]

  5. Without dependabot for now. It seems like a really useful tool, but probably not mission critical. But I guess I will get to looking at it closer too soon. And maybe also at poetry.

pbodnar commented 1 year ago
  1. I have switched to using seemingly the more modern pytest instead of unittest for the automated builds. We'll see how it performs and we could migrate to it fully one day, maybe...?
zyv commented 1 year ago

Just a nitpick:

It's a good style to provide environment variables (especially with secrets) only to the steps that need them. Otherwise they are available to all steps, which could have security implications on one hand, and on the other hand makes it complicated for the readers to understand which steps exactly need them and why.

So I would move the block

env:
  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

to the step level, because my understanding is that it's only needed by coveralls.

Anyways, regarding coveralls, we just enforce the coverage with the corresponding pytest plugin. You don't get a fancy web interface and cool stats, but how interesting are they if the threshold is fixed? And less moving parts is also a good thing...

Poetry is awesome. We have switched years ago from pipenv and haven't looked back.

Congratulations on decommissioning Travis and repairing the CI! 👍

pbodnar commented 1 year ago

@zyv, good point, so I have moved the token.

Regarding the rest, we'll see, thanks for the recommendations! :)