Skip to content

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

Merged
vstinner merged 4 commits into
python:mainfrom
vstinner:regrtest_seq
Jun 3, 2024
Merged

gh-119727: Add --single-process option to regrtest#119728
vstinner merged 4 commits into
python:mainfrom
vstinner:regrtest_seq

Conversation

@vstinner

@vstinnervstinner commented May 29, 2024

Copy link
Copy Markdown
Member

@vstinner

Copy link
Copy Markdown
MemberAuthor

cc @gpshead@Yhg1s

@serhiy-storchakaserhiy-storchaka left a comment

Copy link
Copy Markdown
Member

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.

Comment threadLib/test/libregrtest/cmdline.py Outdated
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
Copy Markdown
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.

NEED_TTY = set('''
test_ioctl
'''.split())
NEED_TTY = {

Copy link
Copy Markdown
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
Copy Markdown
MemberAuthor

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
Copy Markdown
MemberAuthor

@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
Copy Markdown
Member

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

vstinner commented May 31, 2024

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

@vstinner

Copy link
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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
Copy Markdown
MemberAuthor

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

@serhiy-storchaka

Copy link
Copy Markdown
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.

Comment threadMisc/NEWS.d/next/Tests/2024-05-29-15-28-08.gh-issue-119727.dVkaZM.rst Outdated
…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:mainJun 3, 2024
@vstinner
vstinner deleted the regrtest_seq branch June 3, 2024 16:34
@hugovkhugovk changed the title gh-119727: Add --sequentially option to regrtestgh-119727: Add --single-process option to regrtestJun 3, 2024
mliezun pushed a commit to mliezun/cpython that referenced this pull request Jun 3, 2024
barneygale pushed a commit to barneygale/cpython that referenced this pull request Jun 5, 2024
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@miss-islington-app

Copy link
Copy Markdown

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

@miss-islington-app

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Member

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
@bedevere-app

Copy link
Copy Markdown

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

@bedevere-appbedevere-appBot removed the needs backport to 3.13 bugs and security fixes label Aug 14, 2024
@vstinnervstinner added the needs backport to 3.12 only security fixes label Aug 26, 2024
@miss-islington-app

Copy link
Copy Markdown

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

@miss-islington-app

Copy link
Copy Markdown

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

vstinner added a commit that referenced this pull request Aug 26, 2024
…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
…onGH-119728)
(cherry picked from commit 4e8aa32)
Co-authored-by: Victor Stinner <vstinner@python.org>
@bedevere-app

Copy link
Copy Markdown

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

@bedevere-appbedevere-appBot 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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner@hugovk@serhiy-storchaka@mhsmith