Skip to content

gh-119727: Add --single-process option to regrtest #119728

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 3, 2024
Merged

gh-119727: Add --single-process option to regrtest #119728

merged 4 commits into from
Jun 3, 2024

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented May 29, 2024 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request May 29, 2024
Closed
@vstinner
Copy link
Member Author

cc @gpshead @Yhg1s

@vstinner vstinner mentioned this pull request May 29, 2024
Merged
serhiy-storchaka
serhiy-storchaka approved these changes May 30, 2024
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Alternative names are --singleprocess and --no-multiprocess, which more accurately describe the effect, but this is up to you.

I think that the implementation can be simplified by reusing the same destination variable. It is more common in such cases.

Lib/test/libregrtest/cmdline.py Outdated
@@ -307,6 +308,10 @@ def _create_parser():
group.add_argument('-j', '--multiprocess', metavar='PROCESSES',
dest='use_mp', type=int,
help='run PROCESSES processes at once')
group.add_argument('--sequentially', action='store_true',
Copy link
Member

Choose a reason for hiding this comment

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

You can also implement it as action='store_const', dest='use_mp', const=None.

Lib/test/libregrtest/worker.py
NEED_TTY = set('''
test_ioctl
'''.split())
NEED_TTY = {
Copy link
Member

Choose a reason for hiding this comment

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

Not related.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's a follow-up of a previous PR. I didn't want to write a dedicated PR for that.

@vstinner
Copy link
Member Author

@Yhg1s @hugovk: Since the release process requires to run tests sequentially, do you have a preference for the option name?

  • --singleprocess (close to the existing --single option)
  • --no-multiprocess
  • --sequentially
  • --sameprocess
  • something else?

@hugovk
Copy link
Member

hugovk commented May 31, 2024

My first thought is to use something like -j 1 rather than adding a new option. Would this work?

  -j, --multiprocess PROCESSES
                        run PROCESSES processes at once

This is similar to pytest-xdist (-n or --numprocesses) and sphinx-build (-j or --jobs).

@vstinner
Copy link
Member Author

vstinner commented May 31, 2024 •
edited
Loading

My first thought is to use something like -j 1 rather than adding a new option. Would this work?

Currently, -j1 runs tests sequentially but spawn a new process for each test file: see #119727 (comment). I'm open to consider changing this behavior.

@hugovk
Copy link
Member

hugovk commented May 31, 2024

Do you think anyone will realistically want to run sequentially and spawn a new process for each? My guess is not.

@vstinner
Copy link
Member Author

vstinner commented Jun 3, 2024

I'm a little bit worried that some projects currently run tests with -j1 with good reasons, and changing that can lead to surprising results. I would prefer to add a new option.

According to the discussion, the least bad choice is --singleprocess option name :-)

@hugovk
Copy link
Member

hugovk commented Jun 3, 2024

Then I'd include a hyphen for readability/accessibility: --single-process

We have a mixture of options with and without hyphens, so consistency isn't a problem :)

@vstinner
Copy link
Member Author

vstinner commented Jun 3, 2024

I updated the PR to use --single-process name. Are you ok with that @serhiy-storchaka and @hugovk?

@serhiy-storchaka
Copy link
Member

It does not really matter. It was just a tiny nitpick from my side. Use the name that you like if you are sure that it will not create problems in future.

@vstinner @hugovk
Update Misc/NEWS.d/next/Tests/2024-05-29-15-28-08.gh-issue-119727.dVk…
90fb9fe 
…aZM.rst

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@vstinner vstinner enabled auto-merge (squash) June 3, 2024 16:11
@vstinner vstinner merged commit 4e8aa32 into python:main Jun 3, 2024
41 checks passed
@vstinner vstinner deleted the regrtest_seq branch June 3, 2024 16:34
@hugovk hugovk changed the title gh-119727: Add --sequentially option to regrtest gh-119727: Add --single-process option to regrtest Jun 3, 2024
mliezun pushed a commit to mliezun/cpython that referenced this pull request Jun 3, 2024
@vstinner @mliezun
pythongh-119727: Add --single-process option to regrtest (python#119728)
035ef28
barneygale pushed a commit to barneygale/cpython that referenced this pull request Jun 5, 2024
@vstinner @barneygale
pythongh-119727: Add --single-process option to regrtest (python#119728)
713d900
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
@vstinner @noahbkim
pythongh-119727: Add --single-process option to regrtest (python#119728)
dea4632
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@vstinner @estyxx
pythongh-119727: Add --single-process option to regrtest (python#119728)
7dd93f0
@freakboy3742 freakboy3742 mentioned this pull request Aug 14, 2024
Merged
@freakboy3742 freakboy3742 mentioned this pull request Aug 14, 2024
Merged
@mhsmith mhsmith added the needs backport to 3.13 bugs and security fixes label Aug 14, 2024
@miss-islington-app
Copy link

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 4e8aa32245e2d72bf558b711ccdbcee594347615 3.13

@mhsmith mhsmith assigned mhsmith and unassigned vstinner Aug 14, 2024
@mhsmith
Copy link
Member

mhsmith commented Aug 14, 2024

I'll submit a backport of this because it would be useful for the iOS and Android buildbots – #122992 (comment).

mhsmith pushed a commit to mhsmith/cpython that referenced this pull request Aug 14, 2024
@vstinner @mhsmith
pythongh-119727: Add --single-process option to regrtest (python#119728)
4185f2b 
(cherry picked from commit 4e8aa32)
@bedevere-app
Copy link

bedevere-app bot commented Aug 14, 2024

GH-123010 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Aug 14, 2024
@vstinner vstinner added the needs backport to 3.12 only security fixes label Aug 26, 2024
@miss-islington-app
Copy link

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @vstinner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 4e8aa32245e2d72bf558b711ccdbcee594347615 3.12

@miss-islington-app miss-islington-app bot assigned vstinner and unassigned mhsmith Aug 26, 2024
vstinner added a commit that referenced this pull request Aug 26, 2024
@mhsmith @vstinner
[3.13] gh-119727: Add --single-process option to regrtest (#119728) (#…
350313e 
…123010)

gh-119727: Add --single-process option to regrtest (#119728)

(cherry picked from commit 4e8aa32)

Co-authored-by: Victor Stinner <vstinner@python.org>
hugovk pushed a commit to hugovk/cpython that referenced this pull request Feb 20, 2025
@vstinner @hugovk
[3.12] pythongh-119727: Add --single-process option to regrtest (pyth…
ccb4ead 
…onGH-119728)

(cherry picked from commit 4e8aa32)

Co-authored-by: Victor Stinner <vstinner@python.org>
@bedevere-app
Copy link

bedevere-app bot commented Feb 20, 2025

GH-130359 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Feb 20, 2025
hugovk added a commit that referenced this pull request Feb 20, 2025
@hugovk @vstinner
[3.12] gh-119727: Add --single-process option to regrtest (GH-119728) (…
ecfca20 
…#130359)

Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hugovk hugovk hugovk approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

@vstinner vstinner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vstinner @hugovk @serhiy-storchaka @mhsmith