Description

hugovk

opened on Sep 14, 2023

Issue body actions

GitHub Actions is easier to use and maintain. For example, GitHub Actions is more popular and widely used, and so more familiar. It's easier to maintain a single system. Also core devs can restart failed GitHub Actions whereas they can't for Azure Pipelines. Both are owned by Microsoft and I believe they run on much the same infra.

There's a lot of duplication of tests between Azure Pipelines and GitHub Actions.

Let's remove the duplicated tests, and move the unique things over to GitHub Actions. For example, patchcheck is only run on AP.

We might not necessarily need to port things directly over, it may be possible to replicate equivalent checks in another way, possible with a pre-commit or Ruff lint rule. Let's evaluate these separately.

Related issues/PRs:

• Azure Pipelines: macOS CI Tests and Ubuntu CI Tests (coverage) jobs are always skipped #109395
  • gh-109395: Remove skipped macOS builds from Azure Pipelines #109400
  • gh-109395: Remove skipped coverage job from Azure Pipelines #109412
• CI: Remove docs build from Azure Pipelines #105823
• Make Azure Pipelines optional on GitHub PRs #84018
  • bpo-39837: Disable macOS tests on Azure Pipelines #18818

Linked PRs

• gh-109408: Remove Ubuntu unit tests from Azure Pipelines #109452
• gh-109408: Azure Pipelines: test 3.12 branch #109453
• gh-109408: Run patchcheck in GitHub Actions #109459
• [3.12] gh-109408: Remove Ubuntu unit tests from Azure Pipelines (GH-109452) #109519
• [3.11] gh-109408: Remove Ubuntu unit tests from Azure Pipelines (GH-109452) #109520
• [3.12] gh-109408: Azure Pipelines: test 3.12 branch (GH-109453) #109535
• [3.11] gh-109408: Azure Pipelines: test 3.12 branch (GH-109453) #109536
• gh-109408: Move Windows builds from Azure Pipelines PR to GitHub Actions #109569
• [3.12] gh-109408: Move Windows builds from Azure Pipelines PR to GitHub Actions (GH-109569) #109623
• [3.11] gh-109408: Move Windows builds from Azure Pipelines PR to GitHub Actions (GH-109569) #109624
• gh-109408: Add the docs whitespace check from patchcheck to pre-commit #109854
• GH-109408: Move the C file whitespace check from patchcheck to pre-commit #109890
• GH-109408: Move the Python file whitespace check from patchcheck to pre-commit #109891
• gh-109408: Stop running patchcheck in CI #109895
• [3.12] gh-109408: Add the docs whitespace check from patchcheck to pre-commit (GH-109854) #110594
• [3.11] gh-109408: Add the docs whitespace check from patchcheck to pre-commit (GH-109854) #110595
• [3.12] GH-109408: Move the Python file whitespace check from patchcheck to pre-commit (GH-109891) #110633
• [3.12] GH-109408: Move the C file whitespace check from patchcheck to pre-commit (GH-109890) #110636
• [3.11] GH-109408: Move the C file whitespace check from patchcheck to pre-commit (GH-109890) #110640
• [3.11] GH-109408: Move the Python file whitespace check from patchcheck to pre-commit (GH-109891) #110641
• gh-109408: Revert pre-commit whitespace checks pending portable solution #110726
• [3.12] gh-109408: Revert pre-commit whitespace checks pending portable solution (GH-110726) #110730
• GH-109408: Add a make lint target #122333
• GH-109408: Remove .azure-pipelines/pr.yml #122643

Reacted by Sam Gross, sunmy2019 and Erlend E. Aasland

Activity

1. hugovk commented on Sep 14, 2023

   

   hugovk

   on Sep 14, 2023

   MemberAuthor

   More actions

   Hmm, and Azure Pipelines is only running for PRs to main (not backports); and on the version branches after PRs have been merged, but not for 3.12.

   There are two top-level Azure Pipelines config files:

   Azure Pipelines CI

   • Config: .azure-pipelines/ci.yml
   • Builds: https://dev.azure.com/Python/cpython/_build?definitionId=4

   This runs on version branches, after a PR has been merged.

   But when we forked 3.12 from main in May, we forgot to add '3.12' to the config, so it's not run on that branch at all:

   trigger: ['main', '3.11', '3.10', '3.9', '3.8', '3.7']

   Azure Pipelines PR

   • Config: .azure-pipelines/pr.yml
   • Builds: https://dev.azure.com/Python/cpython/_build?definitionId=9

   This runs when PRs are opened. But it's only running for PRs against main, not for any of the backport PRs. I'm not sure why.

   Example

   To illustrate, this set of PRs only only had AP builds for main (python/cpython#109360) and not 3.12 (
   python/cpython#109361) or 3.11 (#109362).

   After merge, AP ran for main (https://dev.azure.com/Python/cpython/_build/results?buildId=137654&view=results) and 3.11 (https://dev.azure.com/Python/cpython/_build/results?buildId=137657&view=results) but not 3.12.

   For comparison, GitHub Actions runs for PRs for all branches, but not merged pushes to version branches.
2. brettcannon commented on Sep 14, 2023

   

   brettcannon

   on Sep 14, 2023

   Member

   More actions

   I believe @zooba said there were some things being tested on Azure Pipelines that GitHub Actions didn't support?
3. zooba commented on Sep 14, 2023

   

   zooba

   on Sep 14, 2023

   Member

   More actions

   Azure Pipelines is doing install layout testing using the APPX layout as a second step post-commit. While I'd most like to have the default build match the install layout, this has been a useful way to catch issues in tests that assume a source directory layout. These tests are templated at https://github.com/python/cpython/blob/main/.azure-pipelines/windows-layout-steps.yml and included a few times in ci.yml.
4. hugovk commented on Sep 14, 2023

   

   hugovk

   on Sep 14, 2023

   MemberAuthor

   More actions

   Looks like we could potentially remove the "Tests" step (and "Publish Test Results") from windows-steps.yml without affecting the layout testing? The main thing in windows-steps.yml being the CPython build.

   In addition to the APPX layout step, should we also keep the nuget and embed layout steps?

   cpython/.azure-pipelines/ci.yml

   Lines 103 to 115 in 3b9d10b

   	steps:
   	- template: ./windows-steps.yml
   	- template: ./windows-layout-steps.yml
   	parameters:
   	kind: nuget
   	- template: ./windows-layout-steps.yml
   	parameters:
   	kind: embed
   	- template: ./windows-layout-steps.yml
   	parameters:
   	kind: appx
   	fulltest: true
5. zooba commented on Sep 14, 2023

   

   zooba

   on Sep 14, 2023

   Member

   More actions

   Yeah, that would work. But it's post-commit testing anyway, so it isn't holding anyone up. And knowing that the regular tests pass is incredibly valuable when something specific to one of the layouts fails.

   I'm pretty sure people are only annoyed by the PR integration. If that goes away, nobody will care unless they're looking for it.

   As for the different layouts, the nuget and embed layouts only do really quick tests. I'd keep them all, as it gives us early warning that something has broken them (e.g. a new DLL isn't being copied). Otherwise we won't find out until during the release process when the same tests are run.

   Reacted by Hugo van Kemenade
6. hugovk commented on Sep 15, 2023

   

   hugovk

   on Sep 15, 2023

   MemberAuthor

   More actions

   Sounds good. I suggest we proceed as follows:

   1. Leave ci.yml be, only change pr.yml
   2. Remove duplicate CI tests from pr.yml, keeping patchcheck on AP
   3. Move patchcheck to GHA, either as is or equivalent lint checks
   4. If necessary, disable "Azure Pipelines PR" via the web UI
   5. Remove pr.yml
7. hugovk commented on Sep 15, 2023

   

   hugovk

   on Sep 15, 2023

   MemberAuthor

   More actions

   Clarification on 1:

   1. Leave Windows testing in ci.yml be, change Windows testing in pr.yml

   Let's remove Ubuntu unit tests from both ci.yml and pr.yml, because they're run on GHA.

   That leaves only patchcheck in pr.yml (it's skipped in ci.yml because it only makes sense for PRs).

   Please see PR #109452.
8. sobolevn

   added a commit that references this issue on Sep 15, 2023

   pythongh-109408: Run `patchcheck` in GitHub Actions

   fd695d5
9. hugovk

   added a commit that references this issue on Sep 17, 2023

   gh-109408: Remove Ubuntu unit tests from Azure Pipelines (#109452)

   a75daed
10. miss-islington

    added a commit that references this issue on Sep 17, 2023

    pythongh-109408: Remove Ubuntu unit tests from Azure Pipelines (pytho…

    4f7e1bb
11. hugovk

    added 2 commits that reference this issue on Sep 17, 2023

    [3.11] pythongh-109408: Remove Ubuntu unit tests from Azure Pipelines (…

    28cceb8

    [3.11] gh-109408: Remove Ubuntu unit tests from Azure Pipelines (GH-1…

    e15b663
12. Yhg1s

    added a commit that references this issue on Sep 17, 2023

    [3.12] gh-109408: Remove Ubuntu unit tests from Azure Pipelines (GH-1…

    5c439fb
13. hugovk

    added a commit that references this issue on Sep 18, 2023

    gh-109408: Azure Pipelines: test 3.12 branch (#109453)

    dd5d214
14. miss-islington

    added a commit that references this issue on Sep 18, 2023

    pythongh-109408: Azure Pipelines: test 3.12 branch (pythonGH-109453)

    8aac67a
15. 26 remaining items
16. AlexWaygood

    added a commit that references this issue on Oct 11, 2023

    [3.12] gh-109408: Revert pre-commit whitespace checks pending portabl…

    c06ac1b
17. iritkatriel

    added

    buildThe build process and cross-buildThe build process and cross-build

    on Dec 1, 2023
18. AA-Turner

    added 2 commits that reference this issue on Aug 3, 2024

    GH-109408: Stop running patchcheck in CI (#109895)

    cc6839a

    gh-109408: Remove ``.azure-pipelines/pr.yml`` (#122643)

    1573d90
19. brandtbucher

    added 2 commits that reference this issue on Aug 7, 2024

    pythonGH-109408: Stop running patchcheck in CI (python#109895)

    ae5e38d

    pythongh-109408: Remove ``.azure-pipelines/pr.yml`` (python#122643)

    e6639de
20. blhsing

    added 2 commits that reference this issue on Aug 22, 2024

    pythonGH-109408: Stop running patchcheck in CI (python#109895)

    fe713fc

    pythongh-109408: Remove ``.azure-pipelines/pr.yml`` (python#122643)

    ea18a92
21. Glyphack

    added 5 commits that reference this issue on Sep 2, 2024

    pythongh-109408: Move Windows builds from Azure Pipelines PR to GitHu…

    210eef5

    pythongh-109408: Add the docs whitespace check from patchcheck to pre…

    a89e045

    pythonGH-109408: Move the Python file whitespace check from patchchec…

    2d6f00d

    pythonGH-109408: Move the C file whitespace check from patchcheck to …

    9e03c53

    pythongh-109408: Revert pre-commit whitespace checks pending portable…

    cedf6ef
22. hugovk commented on Oct 3, 2024

    

    hugovk

    on Oct 3, 2024

    MemberAuthor

    More actions

    Closing, this is essentially done now.

    The remaining Azure Pipelines config is for testing after merge, and can stay to help test packaging the Windows installers.

    We've moved all the pre-merge, PR-level stuff into GitHub Actions.

    #122333 is still open, but isn't directly about moving AP stuff into GHA.

    Reacted by Ege Akman

    Reacted by Ege Akman
23. hugovk

    closed this as completedon Oct 3, 2024

Issue actions