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

Make Windows (x64) GitHub action mandatory on master branch #368

Closed
vstinner opened this issue May 7, 2020 · 14 comments
Closed

Make Windows (x64) GitHub action mandatory on master branch #368

vstinner opened this issue May 7, 2020 · 14 comments

Comments

@vstinner
Copy link
Member

vstinner commented May 7, 2020

Hi,

Last week, I asked to make Travis CI and Windows (x64) CI mandatory on Python PRs: #365

Problem: on documentation-only PRs, GitHub Action jobs were not run but GitHub still required Windows (x64) job to pass even if it never run.

I worked around the issue in the master branch, by always running GitHub action jobs, even for documentation-only PRs:

Would it be possible to make Windows (x64) mandatory in the master branch?

If it goes well, I will backport the change to 3.7 and 3.8 and then request to also make it mandatory there.

Another PR was also proposed to skip GH Actions jobs for documentation-only changes. Basically, re-do what was done previously, but in a different ways (without "paths-ignored") to workaround a GitHub limitation.

@Mariatta @zooba @brettcannon: Is it ok for you?

Example of recent PR: python/cpython#19982 all jobs are green, great :-) (Windows (x64) helped me to find a real bug in the first version of my PR, thanks!)

@vstinner
Copy link
Member Author

@Mariatta @zooba @brettcannon: Friendly ping. Are you ok to make the Windows (x64) CI mandatory on Python PRs?

@Mariatta
Copy link
Member

In general, if the CI is reliable and trustworthy then I think it's good to make it mandatory. But I don't have much context on why it was disabled in the first place (that thread and bpo ticket are a bit long for me to keep up, sorry). I'll defer to the Windows experts for final decision.

@vstinner
Copy link
Member Author

In general, if the CI is reliable and trustworthy then I think it's good to make it mandatory. But I don't have much context on why it was disabled in the first place (that thread and bpo ticket are a bit long for me to keep up, sorry). I'll defer to the Windows experts for final decision.

Previously, it wasn't possible to make a GitHub Action job mandatory because of the issue with documentation only PRs: https://bugs.python.org/issue40548

These jobs were never mandatory. I propose to start to make one mandatory on the master branch.

@vstinner
Copy link
Member Author

Update: GitHub Action Tests jobs are now skipped for documentation-only changes... as it was done before :-) But it's implemented differently, which again, allows to make a job (like Windows x64) mandatory: it should not hold a PR anymore if the job is skipped.
python/cpython@75d7257

@vstinner
Copy link
Member Author

So far, GitHub configuration changes were only pushed into master. I prefer to ensure that it works as expected before backporting changes. But the Windows job cannot be made mandatory in branches other than master until the GitHub configuration changes are backported to these branches.

@Mariatta
Copy link
Member

Perhaps we can first apply and backport the GitHub Actions changes first and see how it works for the next 2 weeks. If it works great, we can then make it a required CI.

Thanks.

@vstinner
Copy link
Member Author

Perhaps we can first apply and backport the GitHub Actions changes first and see how it works for the next 2 weeks. If it works great, we can then make it a required CI.

Sure. I'm fine with this plan.

@vstinner
Copy link
Member Author

@brettcannon @Mariatta: So far, Windows x64 look reliable. I only saw one or two download errors while getting dependencies. Is it now ok to make this CI mandatory?

@vstinner
Copy link
Member Author

vstinner commented Jun 8, 2020

Any update on this issue?

@brettcannon
Copy link
Member

Looking at python/cpython#20694 suggests that Windows x64 is not being run on documentation changes and is still being explicitly skipped instead of being flagged as successful while not running actual tests. I thought the issue we had was GitHub didn't consider skipped tests as passing from a required test perspective. Do we know if that's still the case?

@vstinner
Copy link
Member Author

vstinner commented Jun 8, 2020

Previously, "paths-ignored" was used in the configuration. This caused the troubles.

The workflow was redesigned to use if: needs.check_source.outputs.run_tests == 'true' which should not have the issue.

See https://bugs.python.org/issue40548 for changes and discussion.

@brettcannon
Copy link
Member

I just flipped on the Windows (x64) status check as mandatory on master.

FFY00 added a commit to FFY00/cpython that referenced this issue Jun 8, 2020
Signed-off-by: Filipe Laíns <lains@archlinux.org>
@FFY00
Copy link
Member

FFY00 commented Jun 8, 2020

If you go to python/cpython#20740 you should now see a shiny green button.

I think this can be closed now.

@vstinner
Copy link
Member Author

vstinner commented Jun 8, 2020

If you go to python/cpython#20740 you should now see a shiny green button.

I confirm: the Windows x64 job is skipped but I can still merge the PR :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants