python-can icon indicating copy to clipboard operation
python-can copied to clipboard

CI Strategy EOY 2020

Open hardbyte opened this issue 5 years ago • 13 comments

Travis-CI has been slow of late, although this might improve now we have migrated to travis-ci.com

In #591 I suggested we add support for arm by using Shippable. In #705 @karlding added arm64 support using travis-ci. In #661 @karlding proposed moving to Azure Pipelines. In #940 @felixdivo proposed:

  • retain all Github workflow tests, which test with CPython on Linux, macOS and Windows
  • Move CPython 3.10-dev & nightly from Travis to Github workflow
  • remove Travis CI testing except: pypy3 and socketcan tests
  • retain the other Travis jobs like the linter and publishing (for now at least, we can consider moving them in another PR)
  • remove AppVeyor testing as it is entirely covered by Github workflows (right?)

hardbyte avatar Nov 25 '20 19:11 hardbyte

Azure Pipelines is pretty quick I will say that, and it work well and can be used as a single CI instead of having to use AppVeyor and Travis-CI.

kdschlosser avatar Nov 26 '20 07:11 kdschlosser

Yeah I'm also a fan of Azure, but it seems we can do almost everything with Github actions which would be marginally easier (and probably runs on Azure by now!). AppVeyor has always been pretty slow for us.

Need to check if we can run socketcan tests via github actions.

hardbyte avatar Nov 26 '20 09:11 hardbyte

Ok, so I'll remove some redundant Travis testing (move it to Gitlab action in three lines of code) in #940 and not introduce more on that branch until more have commented on this.

felixdivo avatar Nov 26 '20 09:11 felixdivo

So yeah, travis is faster is it is not longer in queue for half a day.

felixdivo avatar Nov 26 '20 09:11 felixdivo

the GitHub actions are probably running on Azure servers like you said. It is all Microsoft... AppVeyor in it's heyday was good, Just Like Travis-CI was also. They have to much load now and investment into more, newer and faster servers hasn't kept pace. Microsoft has just about unlimited money at their disposal, so it shouldn't end up in the same boat. but who knows

kdschlosser avatar Nov 26 '20 09:11 kdschlosser

It seems like moving to Github actions will not face much resistance here, and it also removes quite a bit of setup complexity for the pipelines. This is because we can (mostly) abandon Travis and completely abandon Appveyor.

I'd thus advocate for merging #940 (@hardbyte you already approved it) and can offer to

  • move Appveyor too,
  • move some more Travis jobs to Github actions (docs & linters)

in a separate PR.

felixdivo avatar Jan 23 '21 10:01 felixdivo

Yeah I'm also a fan of Azure, but it seems we can do almost everything with Github actions which would be marginally easier (and probably runs on Azure by now!). AppVeyor has always been pretty slow for us.

Need to check if we can run socketcan tests via github actions.

I was also curious about using vcan, for example. Github states they don't have plan to support that.

tysonite avatar Jan 30 '21 11:01 tysonite

#940 is merged. This was done:

  • Updates all tools to the currently newest version (please also update your local formatter, etc.)
  • Retain all Github workflow tests, which test with CPython on Linux, macOS and Windows
  • Move CPython 3.10-dev from Travis to Github workflow (remove nightly, which was out of date anyways)
  • Remove Travis CI testing except: ARM and socketcan tests
  • Retain the other Travis jobs like the linter and publishing (for now at least, we can consider moving them in another PR)

To be done:

  • remove AppVeyor (it is now more than covered by GitHub Actions)

felixdivo avatar Apr 15 '21 08:04 felixdivo

After #1009 is merged, IMHO this issue can be closed.

One could think about moving linters, documentation and releasing from Travis CI to GitHub Actions but I don't see a huge benefit there. But also not really a drawback besides the moderate amount of work needed to be done.

felixdivo avatar Apr 15 '21 08:04 felixdivo

@felixdivo @hardbyte Can we use a personal access token here? https://github.com/hardbyte/python-can/blob/84e1f36aee68a0a67c62c9602c1025dd871916b2/.github/workflows/format-code.yml#L28 If the commit is done with the GITHUB_TOKEN, then it does not trigger a new workflow run. But codecov, travis etc are triggered which is messing with the code coverage check. See here for example.

zariiii9003 avatar Apr 17 '21 19:04 zariiii9003

If we switched to using a PAT, what do you suggest to avoid recursive workflow runs?

I'm happy to disable the format-code workflow if it is messing things up

hardbyte avatar Apr 17 '21 21:04 hardbyte

I don't see how it could be recursive. The workflow is only repeated when the black formatting is committed. This cannot happen more than once (unless we do something silly in the future...)

I would change the workflow to:

  1. format
    • check with black formatter
    • if check fails, format with black
    • if check fails commit changes
  2. tests
    • run tests if job 1 finished successfully
example yaml

name: Tests

on: [push, pull_request]

env:
  PY_COLORS: "1"

jobs:
  format:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - name: Set up Python 3.9
        uses: actions/setup-python@v2
        with:
          python-version: 3.9
      - name: Install dependencies
        run: |
          python -m pip install --upgrade pip
          pip install -r requirements-lint.txt
      - name: Code Format Check with Black
        id: black-check
        run: |
          black --check --verbose .
      - name: Format with Black
        id: black-format
        if: ${{ steps.black-check.outcome == 'failure' }}
        run: |
          black --verbose .
      - name: Commit Formatted Code
        if: ${{ steps.black-check.outcome == 'failure' }}
        uses: EndBug/add-and-commit@v5
        env:
          # This is necessary in order to push a commit to the repo
          GITHUB_TOKEN: ${{ secrets.PAT }}
        with:
          message: "Format code with black"
          # Ref https://git-scm.com/docs/git-add#_examples
          add: './*.py'

  test:
    needs: format
    runs-on: ${{ matrix.os }}
    continue-on-error: ${{ matrix.experimental }} # See: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idcontinue-on-error
    strategy:
      matrix:
        os: [ubuntu-latest, macos-latest, windows-latest]
        experimental: [false]
        python-version: [3.6, 3.7, 3.8, 3.9, pypy3]
        include:
          - python-version: 3.10.0-alpha.7  # Newest: https://github.com/actions/python-versions/blob/main/versions-manifest.json
            os: ubuntu-latest
            experimental: true
      fail-fast: false
    steps:
    - uses: actions/checkout@v2
    - name: Set up Python ${{ matrix.python-version }}
      uses: actions/setup-python@v2
      with:
        python-version: ${{ matrix.python-version }}
    - name: Install dependencies
      run: |
        python -m pip install --upgrade pip
        pip install tox
    - name: Test with pytest via tox
      run: |
        tox -e gh

I did not test this of course.

zariiii9003 avatar Apr 17 '21 22:04 zariiii9003

Removing this from the milestone since it does not really have something to do with the release. I just added it to not forget that there were may improvements to the CI setup.

felixdivo avatar Apr 23 '21 22:04 felixdivo

Is there anything to do here?

felixdivo avatar Nov 16 '22 08:11 felixdivo