GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-119727: Add --single-process option to regrtest by vstinner · Pull Request #119728 · python/cpython

Conversation

@vstinner

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Add --single-process option to regrtest to always run tests sequentially (ignore -jN option) #119727

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
pythongh-119727: Add --sequentially option to regrtest
ef0522a
@bedevere-app bedevere-app Bot added the awaiting core review label May 29, 2024
@bedevere-app bedevere-app Bot mentioned this pull request May 29, 2024
Add --single-process option to regrtest to always run tests sequentially (ignore -jN option) #119727
Closed
@vstinner

vstinner commented May 29, 2024

Copy link
Copy Markdown
Member Author

cc @gpshead @Yhg1s
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Change NEED_TTY formatting
46a1e77
@vstinner vstinner mentioned this pull request May 29, 2024
gh-119273: Don't run test_ioctl in a process group #119275
Merged
serhiy-storchaka
serhiy-storchaka approved these changes May 30, 2024
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/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',

@serhiy-storchaka serhiy-storchaka May 30, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/libregrtest/worker.py
NEED_TTY = set('''
test_ioctl
'''.split())
NEED_TTY = {

@serhiy-storchaka serhiy-storchaka May 30, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Not related.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vstinner vstinner Jun 3, 2024

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels May 30, 2024
@vstinner

vstinner commented May 31, 2024

Copy link
Copy Markdown
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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk

hugovk commented May 31, 2024

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).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented May 31, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk

hugovk commented May 31, 2024

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Jun 3, 2024

Copy link
Copy Markdown
Member Author

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 :-)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk

hugovk commented Jun 3, 2024

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 :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner
Rename the option to: --single-process
4d50b87
@vstinner

vstinner commented Jun 3, 2024

Copy link
Copy Markdown
Member Author

I updated the PR to use --single-process name. Are you ok with that @serhiy-storchaka and @hugovk?
hugovk reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented Jun 3, 2024

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
hugovk
hugovk approved these changes Jun 3, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Tests/2024-05-29-15-28-08.gh-issue-119727.dVkaZM.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@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 <[email protected]>
@vstinner
vstinner enabled auto-merge (squash) June 3, 2024 16:11
@vstinner
vstinner merged commit 4e8aa32 into python:main Jun 3, 2024
@vstinner
vstinner deleted the regrtest_seq branch June 3, 2024 16:34
@bedevere-app bedevere-app Bot removed the awaiting merge label Jun 3, 2024
@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
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
This was referenced Aug 14, 2024
Add a --rerun option to the iOS testbed. #122992
Merged
[3.13] Revert use of --rerun in iOS testbed. #122995
Merged
@mhsmith mhsmith added the needs backport to 3.13 only security fixes label Aug 14, 2024
@miss-islington-app

miss-islington-app Bot commented Aug 14, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app

miss-islington-app Bot commented Aug 14, 2024

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app miss-islington-app Bot assigned vstinner Aug 14, 2024
@mhsmith mhsmith assigned mhsmith and unassigned vstinner Aug 14, 2024
@mhsmith

mhsmith commented Aug 14, 2024

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).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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

bedevere-app Bot commented Aug 14, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only 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

miss-islington-app Bot commented Aug 26, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app

miss-islington-app Bot commented Aug 26, 2024

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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 <[email protected]>
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 <[email protected]>
@bedevere-app

bedevere-app Bot commented Feb 20, 2025

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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 <[email protected]>
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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@vstinner vstinner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@vstinner @hugovk @serhiy-storchaka @mhsmith
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.