Skip to content

bpo-32604: Recommit "bpo-32604: PEP 554 for use in test suite (GH-19985)"#20611

Merged
nanjekyejoannah merged 15 commits intopython:masterfrom
nanjekyejoannah:pep_554_test_suite
Jun 10, 2020
Merged

bpo-32604: Recommit "bpo-32604: PEP 554 for use in test suite (GH-19985)"#20611
nanjekyejoannah merged 15 commits intopython:masterfrom
nanjekyejoannah:pep_554_test_suite

Conversation

@nanjekyejoannah
Copy link
Contributor

@nanjekyejoannah nanjekyejoannah commented Jun 3, 2020 •
edited
Loading

Recommit of what was reverted in 9d17cbf

The refleak in the select module was fixed in a separate PR

With this fix, the tests pass:

gitpod /workspace/cpython $ ./python -m test -R 3:3 test_interpreters
0:00:00 load avg: 4.74 Run tests sequentially
0:00:00 load avg: 4.74 [1/1] test_interpreters
beginning 6 repetitions
123456
......
test_interpreters passed in 1 min 9 sec

== Tests result: SUCCESS ==

1 test OK.

Total duration: 1 min 9 sec
Tests result: SUCCESS

https://bugs.python.org/issue32604

@vstinner
Copy link
Member

vstinner commented Jun 3, 2020

Sorry, I don't have the bandwidth to review this PR.

@nanjekyejoannah
Copy link
Contributor Author

nanjekyejoannah commented Jun 3, 2020

The contents of this PR were already reviewed by @ericsnowcurrently . The only confirmation here is to confirm that no leaks make the tests fail which I already did. There are no changes to what had been merged before

@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 3, 2020
@bedevere-bot
Copy link

bedevere-bot commented Jun 3, 2020

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 9577395 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 3, 2020
pablogsal
pablogsal reviewed Jun 3, 2020
View reviewed changes
@pablogsal
Copy link
Member

pablogsal commented Jun 3, 2020 •
edited
Loading

I didn't review this but I am confused....why there is an rst file with documentation in the test directory?

@nanjekyejoannah
Copy link
Contributor Author

nanjekyejoannah commented Jun 3, 2020 •
edited
Loading

@pablogsal, I think the discussion around this was Eric said we needed to have a reference to the added API in test.support . We could not add the document file in Doc/library/* before the API moved to stdlib. You can follow the discussion here: #19985

Edit:

I have looked and do not see docs anywhere else other than the Docs folder though. So am okay removing the .rst for this PR until consensus is reached.

@nanjekyejoannah
Copy link
Contributor Author

nanjekyejoannah commented Jun 5, 2020

@pablogsal I decided to remove the docs. I hope this is okay now.

ericsnowcurrently
ericsnowcurrently approved these changes Jun 5, 2020
View reviewed changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM As I said when I reviewed the original PR, this is a good start. :) Thanks for doing this.

@nanjekyejoannah nanjekyejoannah merged commit bae872f into python:master Jun 10, 2020
@ericsnowcurrently ericsnowcurrently mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal pablogsal left review comments

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes

@vstinner vstinner Awaiting requested review from vstinner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@nanjekyejoannah @vstinner @bedevere-bot @pablogsal @ericsnowcurrently @the-knights-who-say-ni