Skip to content

gh-109276: regrtest: shorter list of resources - #110326

Merged
vstinner merged 2 commits into
python:mainfrom
vstinner:regrtest_resources
Oct 4, 2023
Merged

gh-109276: regrtest: shorter list of resources#110326
vstinner merged 2 commits into
python:mainfrom
vstinner:regrtest_resources

Conversation

@vstinner

@vstinnervstinner commented Oct 4, 2023

Copy link
Copy Markdown
Member

@vstinner

Copy link
Copy Markdown
MemberAuthor

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

@vstinner

Copy link
Copy Markdown
MemberAuthor

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

@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.

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.

Comment threadLib/test/libregrtest/utils.py Outdated
@vstinner

Copy link
Copy Markdown
MemberAuthor

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.

@vstinner
vstinner enabled auto-merge (squash) October 4, 2023 09:07
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}')

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.

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

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.

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

@vstinner
vstinner merged commit efd8c7a into python:mainOct 4, 2023
@vstinner
vstinner deleted the regrtest_resources branch October 4, 2023 09:39
@vstinner

Copy link
Copy Markdown
MemberAuthor

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.

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@vstinner@serhiy-storchaka