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-109276: regrtest: shorter list of resources by vstinner · Pull Request #110326 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Oct 4, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • Issue: Enhance Python regrtest test runner (test.libregrtest) #109276

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-app bedevere-app Bot added the awaiting review label Oct 4, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Oct 4, 2023
Enhance Python regrtest test runner (test.libregrtest) #109276
Closed
@vstinner
vstinner force-pushed the regrtest_resources branch from 1376d46 to 73ad79e Compare October 4, 2023 06:46
@vstinner
pythongh-109276: regrtest: shorter list of resources
68f2ade
@vstinner
vstinner force-pushed the regrtest_resources branch from 73ad79e to 68f2ade Compare October 4, 2023 06:54
@vstinner

vstinner commented Oct 4, 2023

Copy link
Copy Markdown
Member Author

Examples:

$ ./python -m test -u network
== resources (1): network

$ ./python -m test --slow-ci
== resources: all

$ ./python -m test --fast-ci
== resources: all, -cpu


$ ./python -m test -u all,tzdata
== resources: all, +tzdata

$ ./python -m test 
== resources: all test resources are disabled, use -u option to unskip tests
All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner added the skip news label Oct 4, 2023
@vstinner

vstinner commented Oct 4, 2023

Copy link
Copy Markdown
Member Author

@serhiy-storchaka: Does it look better like that?

All reactions

Sorry, something went wrong.

Uh oh!

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

serhiy-storchaka
serhiy-storchaka reviewed Oct 4, 2023
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.

I did not have problems with the older output, but if you want to change it, the new one looks better.

Did you consider the idea of making it compatible with the input to the -u option? I.e. remove space after comma and remove plus sign. Alternatively you can make -u accepting input with spaces and pluses (although copying text with spaces is less convenient). Not that I personally need this feature, but it might come in handy one day.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/test/libregrtest/utils.py Resolved

Uh oh!

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

@vstinner
Address Serhiy's review
63e0afe
@vstinner

vstinner commented Oct 4, 2023

Copy link
Copy Markdown
Member Author

Did you consider the idea of making it compatible with the input to the -u option? I.e. remove space after comma and remove plus sign.

Done.

Use sorted() for deterministic result.

Done.

All reactions

Sorry, something went wrong.

Uh oh!

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

@vstinner
vstinner enabled auto-merge (squash) October 4, 2023 09:07
serhiy-storchaka
serhiy-storchaka approved these changes Oct 4, 2023
View reviewed changes
Comment thread Lib/test/libregrtest/utils.py
for name in sorted(all_resources - use_resources):
relative_all.append(f'-{name}')
for name in sorted(use_resources - all_resources):
relative_all.append(f'{name}')

@serhiy-storchaka serhiy-storchaka Oct 4, 2023

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.

Suggested change
relative_all.append(f'{name}')
relative_all.append(name)

Sorry, something went wrong.

Uh oh!

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

All reactions

@vstinner vstinner Oct 4, 2023

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.

Oh right. Sadly, I saw your comment after the change was merged. I make try to include it in my next regrtest change ;-)

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 core review and removed awaiting review labels Oct 4, 2023
@vstinner
vstinner merged commit efd8c7a into python:main Oct 4, 2023
@vstinner
vstinner deleted the regrtest_resources branch October 4, 2023 09:39
@bedevere-app bedevere-app Bot removed the awaiting core review label Oct 4, 2023
@vstinner

vstinner commented Oct 4, 2023

Copy link
Copy Markdown
Member Author

I did not have problems with the older output

For me, it's not easy to know if it's default --slow-ci option, default --fast-ci option, or if -u options were modified on a buildbot. With the new output relative to -u all, it should be easier to guess the test config.

All reactions

Sorry, something went wrong.

Uh oh!

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

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@vstinner @Glyphack
pythongh-109276: regrtest: shorter list of resources (python#110326)
5a3e395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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.

2 participants

@vstinner @serhiy-storchaka
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.