Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#11628 Run Windows CI tests on GitHub Actions. #11629

Merged
merged 14 commits into from Sep 1, 2022
Merged
87 changes: 73 additions & 14 deletions .github/workflows/test.yaml
@@ -1,5 +1,9 @@
# Try to get a short workflow name and a job name that start with Python
# version to make it easier to check the status inside GitHub UI.
#
# When using external actions check that the extenral repos are permitted via
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in "external" here

# the GitHub configuration at https://github.com/twisted/twisted/settings/actions
#
exarkun marked this conversation as resolved.
Show resolved Hide resolved
name: CI

on:
Expand All @@ -24,22 +28,42 @@ defaults:
run:
shell: bash

env:
# Set to 'yes' to open a tunnel to GitHub's VMs through tmate on failures.
TMATE_DEBUG: 'no'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only want to enable tmate when we are to start a debug session...and not on any failure.

tmate is only triggered on failure


jobs:
testing:
runs-on: ubuntu-20.04
name: ${{ matrix.python-version }}-${{ matrix.job-name || 'lnx-default-tests' }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "linux" will be more clear in the UI than "lnx" and I don't think the extra 2 letters will make the names so long it becomes a problem.

# We have Ubuntu as the base for running agains multiple Python versions.
runs-on: "${{ matrix.runs-on || 'ubuntu-20.04'}}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the 'ubuntu-20.04' is not explicitly defined in the matrix so that we don't have to repeat it each time we extend it.

We leave it empty and use a default value.

Another option is to define the default values via global environment variables.

env:
   DEFAULT_RUNS_ON: 'ubuntu-22.04'
jobs:
  testing:
    runs-on: "${{ matrix.runs-on || 'env.DEFAULT_RUNS_ON'}}"

env:
TOXENV: "${{ matrix.tox-env }}"
# By default we run all tests with all deps with coverage.
TOXENV: "${{ matrix.tox-env || 'alldeps-withcov-posix' }}"
# As of April 2021 GHA VM have 2 CPUs - Azure Standard_DS2_v2
# Trial distributed jobs enabled to speed up the CI jobs.
TRIAL_ARGS: "-j 4"
name: ${{ matrix.python-version }}${{ matrix.noipv6 }}-${{ matrix.tox-env }}
# On Windows, we don't yet enable distributed tests as is not yet
# supported.
TRIAL_ARGS: "${{ matrix.trial-args || '-j 4' }}"
strategy:
fail-fast: false
# The matrix is designed to run by default various Python versions with
# the default OS and tests suite.
# Default values should be declared via empty string, to allow them
# to be omitted when extending the matrix.
matrix:
# Run on latest minor release of each major python version.
# Run on latest minor release of each major Python version.
python-version: ['3.8', '3.9', '3.10']
tox-env: ['alldeps-withcov-posix']
# Just use the default OS.
runs-on: ['']
# Human readable short description for this job.
job-name: ['']
# This is the main tox target.
# It is later extended with more jobs that should run in a specific
# OS + Python version configuration.
tox-env: ['']
# We just go with the default arguments.
trial-args: ['']
# By default, tests are executed without disabling IPv6.
noipv6: ['']
# By default, tests are executed without extra platform dependencies.
Expand All @@ -48,14 +72,34 @@ jobs:
tox-wrapper: ['']
# Tests are executed with the default target which is the full test suite.
trial-target: ['']
# By default the coverate is not skipped.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in "coverage" here

skip-coverage: ['']

# Here the matrix is extended with variations of the default
# value.
include:
# Windows, minimum Python version with select reactor.
- python-version: '3.7'
runs-on: 'windows-2022'
tox-env: 'alldeps-withcov-windows'
job-name: 'win-default-tests-select'
# Distributed trial is not yet suported on Windows so we overwrite
# the default trial-args to remove concurrent runs and to
# select a reactor.
trial-args: '--reactor=select'
# Windows, latest Python version with iocp reactor.
- python-version: '3.10'
runs-on: 'windows-2022'
tox-env: 'alldeps-withcov-windows'
job-name: 'win-default-tests-iocp'
# Distributed trial is not yet suported on Windows.
trial-args: '--reactor=iocp'
# `noipv6` is created to make sure all is OK on an OS which doesn't
# have IPv6 available.
# Any supported Python version is OK for this job.
- python-version: '3.7'
tox-env: alldeps-withcov-posix
noipv6: -noipv6
job-name: 'default-tests-noipv6'
# `nodeps` is created to make sure we don't have import errors in the
# runtime and production code.
# The minimum supported Python version should be used to maximize
Expand All @@ -66,23 +110,27 @@ jobs:
# `python_requires` from `setup.cfg`.
- python-version: '3.7.1'
tox-env: nodeps-withcov-posix
job-name: 'nodeps-test'
# On PYPY coverage is very slow (5min vs 30min) as there is no C
# extension.
# There is very little PYPY specific code so there is not much to
# gain from reporting coverage.
- python-version: pypy-3.7
tox-env: alldeps-nocov-posix
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- python-version: pypy-3.7
tox-env: alldeps-nocov-posix
- python-version: 'pypy-3.7'
tox-env: 'alldeps-nocov-posix'

job-name: 'pypy-no-coverage'
skip-coverage: yes
# We still run some tests with coverage,
# as there are test with specific code branches for pypy.
- python-version: pypy-3.7
tox-env: alldeps-withcov-posix
trial-target: twisted.test.test_compat twisted.test.test_defer twisted.internet.test.test_socket twisted.trial.test.test_tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- python-version: pypy-3.7
tox-env: alldeps-withcov-posix
trial-target: twisted.test.test_compat twisted.test.test_defer twisted.internet.test.test_socket twisted.trial.test.test_tests
- python-version: 'pypy-3.7'
trial-target: 'twisted.test.test_compat twisted.test.test_defer twisted.internet.test.test_socket twisted.trial.test.test_tests'

job-name: 'pypy-coverage'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We get some redundancy in the name from this - eg "pypy-3.7-alldeps-nocov-posix". Maybe 'coverage' would be better?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. I went for explicit 'with-coverage'

# We run selected test for GTK reactor inside X virtual framebuffer.
- python-version: '3.7'
tox-env: alldeps-gtk-withcov-posix
trial-target: twisted.internet.test
platform-deps: 'gtk_platform'
tox-wrapper: 'xvfb-run -a'
job-name: 'gtk-tests'


steps:
Expand All @@ -99,7 +147,7 @@ jobs:
echo "::set-output name=dir::$(pip cache dir)"

- name: pip cache
uses: actions/cache@v2
uses: actions/cache@v3
exarkun marked this conversation as resolved.
Show resolved Hide resolved
with:
path: ${{ steps.pip-cache.outputs.dir }}
key:
Expand All @@ -108,6 +156,8 @@ jobs:
restore-keys: |
${{ runner.os }}-pip-

# Make sure the matrix is defined in such a way that this is triggered
# only on Linux.
- name: Disable IPv6
if: matrix.noipv6
run: |
Expand All @@ -121,6 +171,8 @@ jobs:
run: |
python -m pip install --upgrade pip tox coverage

# Make sure the matrix is defined in such a way that this is triggered
# only on Linux.
- name: Install GTK system deps
if: matrix.platform-deps == 'gtk_platform'
run: |
Expand All @@ -139,8 +191,15 @@ jobs:
run: |
${{ matrix.tox-wrapper }} tox ${{ matrix.trial-target }}

# If one of the above steps fails, fire up tmate for remote debugging.
- name: Tmate debug on failure
if: ${{ !cancelled() && env.TMATE_DEBUG == 'yes' }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflow for using this action, then, will be to change TMATE_DEBUG at the top of the file to 'yes' and re-run the build, then change it back to 'no' again afterwards?

I guess there's a good reason not to have it turned on all the time? https://github.com/mxschmitt/action-tmate#manually-triggered-debug looks like a slightly nicer workflow, maybe it's not too much more work to do it that way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, editing the text was easier than using the Github action manual workflow trigger.

It is true that you can forget to set it back to no afterward.


You don't want it turned on all the time, as then on failure it will "hang" and wait for someone to connect via SSH.


I will push a commit that will also add manual triggers.
Not sure what are the permissions for manual triggers... that is, if you can trigger from a fork.

uses: mxschmitt/action-tmate@v3
with:
limit-access-to-actor: true

- name: Prepare coverage
if: ${{ !cancelled() && contains(matrix['tox-env'], 'withcov') }}
if: ${{ !cancelled() && !matrix.skip-coverage }}
exarkun marked this conversation as resolved.
Show resolved Hide resolved
run: |
# sub-process coverage are generated in separate files so we combine them
# to get an unified coverage for the local run.
Expand All @@ -149,11 +208,11 @@ jobs:
python -m coverage xml -o coverage.xml -i
python -m coverage report --skip-covered

- uses: codecov/codecov-action@v2
if: ${{ !cancelled() && contains(matrix['tox-env'], 'withcov') }}
- uses: codecov/codecov-action@v3
if: ${{ !cancelled() && !matrix['skip-coverage'] }}
with:
files: coverage.xml
name: lnx-${{ matrix.python-version }}-${{ matrix.tox-env }}${{ matrix.noipv6 }}
name: ${{ matrix.python-version }}-${{matrix.job-name || 'lnx-default-tests' }}
fail_ci_if_error: true


Expand Down Expand Up @@ -278,7 +337,7 @@ jobs:

- name: Publish to PyPI - on tag
if: startsWith(github.ref, 'refs/tags/twisted-')
uses: pypa/gh-action-pypi-publish@v1.3.1
uses: pypa/gh-action-pypi-publish@v1.5.1
with:
password: ${{ secrets.PYPI_UPLOAD_TOKEN }}

Expand Down
2 changes: 1 addition & 1 deletion azure-pipelines/run_test_steps.yml
Expand Up @@ -65,7 +65,7 @@ steps:
displayName: 'Run tests'
continueOnError: ${{ parameters.allowFailure }}
env:
TWISTED_REACTOR: ${{ parameters.windowsReactor }}
TRIAL_ARGS: --reactor=${{ parameters.windowsReactor }}
exarkun marked this conversation as resolved.
Show resolved Hide resolved

- bash: |
# sub-process coverage are generated in separate files so we combine them
Expand Down
Empty file.
4 changes: 2 additions & 2 deletions tox.ini
Expand Up @@ -93,12 +93,12 @@ commands =
posix: python -c "print('Running on POSIX (no special dependencies)')"

; Run tests without wrapping them using coverage.
nocov: python -m twisted.trial --temp-directory={envtmpdir}/_trial_temp --reactor={env:TWISTED_REACTOR:default} --reporter={env:TRIAL_REPORTER:verbose} {env:TRIAL_ARGS:} {posargs:twisted}
nocov: python -m twisted.trial --temp-directory={envtmpdir}/_trial_temp --reporter={env:TRIAL_REPORTER:verbose} {env:TRIAL_ARGS:} {posargs:twisted}
exarkun marked this conversation as resolved.
Show resolved Hide resolved

; Run the tests wrapped using coverage.
withcov: python {toxinidir}/admin/_copy.py {toxinidir}/admin/zz_coverage.pth {envsitepackagesdir}/zz_coverage.pth
withcov: coverage erase
withcov: coverage run -p --rcfile={toxinidir}/.coveragerc -m twisted.trial --temp-directory={envtmpdir}/_trial_temp --reactor={env:TWISTED_REACTOR:default} --reporter={env:TRIAL_REPORTER:verbose} {env:TRIAL_ARGS:} {posargs:twisted}
withcov: coverage run -p --rcfile={toxinidir}/.coveragerc -m twisted.trial --temp-directory={envtmpdir}/_trial_temp --reporter={env:TRIAL_REPORTER:verbose} {env:TRIAL_ARGS:} {posargs:twisted}

lint: pre-commit {posargs:run --all-files --show-diff-on-failure}

Expand Down