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

adiroiban · 2022-08-30T16:45:44Z

Scope and purpose

Fixes #11628

See inline comments.

Besides enabling the Windows tests I have also enabled tmate to allow debugging Windows.

This is the main reason why I have moved the Windows tests to GHA.

…ut on windows. Add debug action.

adiroiban

I think this is ready for an initial review

A previous run at https://github.com/twisted/twisted/runs/8099083260?check_suite_focus=true

needs-review

.github/workflows/test.yaml

adiroiban · 2022-08-30T18:38:49Z

.github/workflows/test.yaml

+  # Set to 'yes' to open a tunnel to GitHub's VMs through tmate on failures.
+  # Also increase timeout-minutes for the relevant OS when debugging remotely
+  # and you need a longer session.
+  TMATE_DEBUG: 'no'


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

adiroiban · 2022-08-30T18:40:24Z

.github/workflows/test.yaml

-    runs-on: ubuntu-20.04
+    name: ${{ matrix.python-version }}-${{ matrix.job-name || 'lnx-default-tests' }}
+    # We have Ubuntu as the base for running agains multiple Python versions.
+    runs-on: "${{ matrix.runs-on || 'ubuntu-20.04'}}"


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'}}"

azure-pipelines/run_test_steps.yml

tox.ini

adiroiban · 2022-08-30T18:47:26Z

please review

exarkun

Thanks (and for talking me through it on IRC too)! This looks like a great start. I pointed out a few minor things inline.

exarkun · 2022-08-30T18:53:34Z

.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


typo in "external" here

.github/workflows/test.yaml

tox.ini

azure-pipelines/run_test_steps.yml

exarkun · 2022-08-30T19:09:01Z

.github/workflows/test.yaml


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


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.

exarkun · 2022-08-30T19:12:07Z

.github/workflows/test.yaml

@@ -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.


Typo in "coverage" here

exarkun · 2022-08-30T19:30:32Z

.github/workflows/test.yaml

            trial-target: twisted.test.test_compat twisted.test.test_defer twisted.internet.test.test_socket twisted.trial.test.test_tests
+            job-name: 'pypy-coverage'


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

yes. I went for explicit 'with-coverage'

exarkun · 2022-08-30T19:32:44Z

.github/workflows/test.yaml

@@ -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' }}


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?

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.

.github/workflows/test.yaml

adiroiban · 2022-08-30T20:19:50Z

I hope I have addressed all the comments.

please review

exarkun

Thanks! This looks quite good. I left some minor suggestions inline. Take them if you like.

One thing I was not able to do was convince myself that 3.7-win-default-tests-select is actually running the tests against Python 3.7 and 3.10-win-default-tests-iocp is testing with Python 3.10. Can you point at anything in the logs that reveals this? Or maybe add something to the CI jobs somewhere that makes this clear. Then I'm happy to see this merged.

.github/workflows/test.yaml

exarkun · 2022-08-31T13:54:30Z

.github/workflows/test.yaml

          - python-version: pypy-3.7
            tox-env: alldeps-nocov-posix


Suggested change

- python-version: pypy-3.7

tox-env: alldeps-nocov-posix

- python-version: 'pypy-3.7'

tox-env: 'alldeps-nocov-posix'

exarkun · 2022-08-31T13:55:24Z

.github/workflows/test.yaml

          - 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


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'

.github/workflows/test.yaml

exarkun · 2022-08-31T14:06:13Z

.github/workflows/test.yaml

+        type: boolean
+        description: 'Run the build with tmate debugging enabled (https://github.com/marketplace/actions/debugging-with-tmate)'
+        required: false
+        default: false


Cool. No doubt the only way to know if this actually works is to merge the PR and then try clicking buttons on GitHub Actions ... I'll certainly try that out as soon as this lands.

Yes. The GitHub UI is updated only after merge.

But you try the TMATE_DEBUG: 'yes' version.
That version is also ok for debugging the process itself.
So if the UI trigger doesn't work, you can force tmate.

adiroiban · 2022-09-01T09:39:18Z

Thanks for the review. I will get this merged.

I saw a codecov failure ... so macOS should be next, and then try to run coverage report on GHA.

When tox runs, in any environment, local or remote-ci you will see the python version

See the link https://github.com/twisted/twisted/runs/8102345847?check_suite_focus=true#step:10:36

tox code at

twisted/tox.ini

Lines 86 to 91 in 26c3c04

    
           commands = 
        
               ; 
        
               ; Display information about Python interpreter 
        
               ; which will be used in subsequent steps 
        
               ; 
        
               python {toxinidir}/admin/dump_all_version_info.py

Local run is something like this

$ tox -e alldeps-nocov 
alldeps-nocov wheel-make: cleaning up build directory ...
alldeps-nocov wheel-make: commands[0] | pip wheel /home/adi/chevah/twisted --no-deps --use-pep517 --wheel-dir /home/adi/chevah/twisted/.tox/dist
Processing /home/adi/chevah/twisted
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: Twisted
  Building wheel for Twisted (pyproject.toml) ... done
  Created wheel for Twisted: filename=Twisted-22.4.0.post0-py3-none-any.whl size=3102521 
- [SNIP}
alldeps-nocov run-test-pre: PYTHONHASHSEED='544835508'
alldeps-nocov run-test: commands[0] | python /home/adi/chevah/twisted/admin/dump_all_version_info.py
+::group::Python info
+/home/adi/chevah/twisted/.tox/alldeps-nocov
+/home/adi/chevah/twisted/.tox/alldeps-nocov
+/home/adi/chevah/twisted/.tox/alldeps-nocov/bin/python
+3.10.4 (main, Jun 29 2022, 12:14:53) [GCC 11.2.0]
+linux
+::endgroup::
alldeps-nocov run-test: commands[1] | python -m twisted.trial --temp-directory=/home/adi/chevah/twisted/.tox/alldeps-nocov/tmp/_trial_temp --reporter=verbose twisted

adiroiban · 2022-09-01T10:06:49Z

.github/workflows/test.yaml

+
          # 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


Suggested change

# 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

# 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'

Ok. This goes to the "delete-lines" and can't be applied by github.
Will manually apply it.

Co-authored-by: Jean-Paul Calderone <exarkun@twistedmatrix.com>

adiroiban force-pushed the 11628-win-ci-on-gha branch from 2f6019a to 8403983 Compare August 30, 2022 16:47

Initial move to run Win tests on GHA.

4f93bcb

adiroiban force-pushed the 11628-win-ci-on-gha branch from 8403983 to 4f93bcb Compare August 30, 2022 17:11

adiroiban added 2 commits August 30, 2022 18:22

Refactor default job values.

4184e07

See if the stars were alligned.

4e0a75a

adiroiban force-pushed the 11628-win-ci-on-gha branch from 0544907 to 4e0a75a Compare August 30, 2022 17:35

adiroiban added 2 commits August 30, 2022 18:46

Pass reactor via generic trial args.

5e070db

Fix skip coverage. Use explicit job name.

d69db98

adiroiban force-pushed the 11628-win-ci-on-gha branch 2 times, most recently from 701c4c8 to eed44da Compare August 30, 2022 18:20

Pin external deps actions. Update cache in the hope it will not timeo…

4896d48

…ut on windows. Add debug action.

adiroiban force-pushed the 11628-win-ci-on-gha branch from eed44da to 4896d48 Compare August 30, 2022 18:22

adiroiban added 2 commits August 30, 2022 19:29

Fix external actions versions.

b70bb27

Fix pypi version.

a7bb10e

adiroiban commented Aug 30, 2022

View reviewed changes

adiroiban mentioned this pull request Aug 30, 2022

[#33] Address Twisted needs chevah/github-hooks-server#43

Merged

3 tasks

chevah-robot added the needs-review label Aug 30, 2022

chevah-robot requested a review from a team August 30, 2022 18:47

exarkun self-assigned this Aug 30, 2022

exarkun requested changes Aug 30, 2022

View reviewed changes

chevah-robot added needs-changes and removed needs-review labels Aug 30, 2022

Define all matrix jobs via include.

07a8125

adiroiban force-pushed the 11628-win-ci-on-gha branch from eec397c to 08418b2 Compare August 30, 2022 20:02

Fix typos and env usage add manual trigger.

f5662eb

adiroiban force-pushed the 11628-win-ci-on-gha branch from 08418b2 to f5662eb Compare August 30, 2022 20:03

Hack the matrix.

520de93

chevah-robot added the needs-review label Aug 30, 2022

chevah-robot removed the needs-changes label Aug 30, 2022

chevah-robot requested a review from a team August 30, 2022 20:19

Remove win tests from Azure pipelines.

528c3a6

adiroiban force-pushed the 11628-win-ci-on-gha branch from 05e5c85 to 528c3a6 Compare August 30, 2022 21:49

adiroiban mentioned this pull request Aug 31, 2022

run with windows on GHA #1647

Closed

7 tasks

exarkun approved these changes Aug 31, 2022

View reviewed changes

chevah-robot added needs-merge and removed needs-review labels Aug 31, 2022

exarkun assigned adiroiban and unassigned exarkun Aug 31, 2022

adiroiban commented Sep 1, 2022

View reviewed changes

adiroiban and others added 2 commits September 1, 2022 11:08

[no ci] Typos and explicit string yaml markup.

0043ce3

Co-authored-by: Jean-Paul Calderone <exarkun@twistedmatrix.com>

Use explicit string markup,

865eaa8

adiroiban enabled auto-merge September 1, 2022 10:24

adiroiban merged commit f4c9fbe into trunk Sep 1, 2022

adiroiban deleted the 11628-win-ci-on-gha branch September 1, 2022 10:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

adiroiban commented Aug 30, 2022 •

edited

adiroiban left a comment

adiroiban Aug 30, 2022

adiroiban Aug 30, 2022

adiroiban commented Aug 30, 2022

exarkun left a comment

exarkun Aug 30, 2022

exarkun Aug 30, 2022

exarkun Aug 30, 2022

exarkun Aug 30, 2022

adiroiban Aug 30, 2022

exarkun Aug 30, 2022

adiroiban Aug 30, 2022

adiroiban commented Aug 30, 2022

exarkun left a comment

exarkun Aug 31, 2022

exarkun Aug 31, 2022

exarkun Aug 31, 2022

adiroiban Sep 1, 2022

adiroiban commented Sep 1, 2022

adiroiban Sep 1, 2022

adiroiban Sep 1, 2022

		trial-target: twisted.test.test_compat twisted.test.test_defer twisted.internet.test.test_socket twisted.trial.test.test_tests
		job-name: 'pypy-coverage'

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

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

Conversation

adiroiban commented Aug 30, 2022 • edited

Scope and purpose

adiroiban left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adiroiban commented Aug 30, 2022

exarkun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adiroiban commented Aug 30, 2022

exarkun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adiroiban commented Sep 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adiroiban commented Aug 30, 2022 •

edited