◐ Shell
reader mode source ↗
Skip to content

bpo-34279: Issue a warning if no tests have been executed#10150

Merged
vstinner merged 5 commits into
python:masterfrom
pablogsal:no_test_run
Nov 29, 2018
Merged

bpo-34279: Issue a warning if no tests have been executed#10150
vstinner merged 5 commits into
python:masterfrom
pablogsal:no_test_run

Conversation

@pablogsal

@pablogsal pablogsal commented Oct 27, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@pablogsal pablogsal self-assigned this Oct 27, 2018
@pablogsal pablogsal requested a review from vstinner October 27, 2018 12:28
@pablogsal pablogsal force-pushed the no_test_run branch 3 times, most recently from dc0da37 to c684dd4 Compare October 27, 2018 12:51
vstinner
vstinner requested changes Oct 28, 2018
View reviewed changes

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

Hide comment

I suggest to add a list of tests which ran no tests to the Regrtest class, so it becomes possible to list them in display_result().

It would be better if test_sys is listed in the final report when running: "./python -m test -j0 test_os test_sys -m test_access". Currently, you have to read all logs to notice that test_sys ran no tests.

@bedevere-bot

bedevere-bot commented Oct 28, 2018

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

And if you don't make the requested changes, you will be poked with soft cushions!

@vstinner

vstinner commented Nov 28, 2018

Copy link
Copy Markdown
Member 
./python -m test -j0 test_os test_sys -m test_access
Tests result: SUCCESS, NO TEST RUN

This command runs test_os.test_access(), it does not run "no test": it runs one test :-) Maybe we should only display NO TEST RUN is 0 test has been run? So display "SUCCESS" on the first example.

NO TEST RUN would make sense on such example:

./python -m test -j0 test_os test_sys -m test_blech
Tests result: NO TEST RUN

Sorry if I asked something different, I wasn't sure of what I want :-)

"Tests result: NO TEST RUN" means "you did something wrong", whereas "./python -m test -j0 test_os test_sys -m test_access" is not a mistake: it can be deliberate to run 0 test in test_sys. This is especially true when you use "./python -m test.bisect".

@pablogsal

pablogsal commented Nov 28, 2018

Copy link
Copy Markdown
Member Author 
./python -m test -j0 test_os test_sys -m test_access
Tests result: SUCCESS, NO TEST RUN

This command runs test_os.test_access(), it does not run "no test": it runs one test :-) Maybe we should only display NO TEST RUN is 0 test has been run? So display "SUCCESS" on the first example.

NO TEST RUN would make sense on such example:

./python -m test -j0 test_os test_sys -m test_blech
Tests result: NO TEST RUN

Sorry if I asked something different, I wasn't sure of what I want :-)

"Tests result: NO TEST RUN" means "you did something wrong", whereas "./python -m test -j0 test_os test_sys -m test_access" is not a mistake: it can be deliberate to run 0 test in test_sys. This is especially true when you use "./python -m test.bisect".

That was the previous behaviour :)

Basically, without commit 2272ba8 this is the situation:

❯ ./python -m test -j0 test_os test_sys -m test_access
Tests result: SUCCESS

❯ ./python -m test -j0 test_os test_sys -m test_blech
Tests result: NO TEST RUN

❯ ./python -m test -j0 test_os -m test_blech
Tests result: NO TEST RUN

❯ ./python -m test -j0 test_os -m test_access
Tests result: SUCCESS

I reverted 2272ba8 in 9c7e7b5, so this PR is how it was before it (as in the example in this message).

vstinner
vstinner approved these changes Nov 29, 2018
View reviewed changes

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

Hide comment

LGTM, but you may move the NEWS entry to the Tests category.

@vstinner vstinner merged commit 9724348 into python:master Nov 29, 2018
@bedevere-bot bedevere-bot removed the label Nov 29, 2018
@vstinner vstinner mentioned this pull request Nov 29, 2018
Merged
vstinner added a commit that referenced this pull request Nov 29, 2018
@vstinner
[3.7] bpo-34279: Synchronize regrtest with master (GH-10800) …
8a73cac 
* bpo-34605, libregrtest: Rename --slaveargs to --worker-args (GH-9099)

Rename also run_tests_slave() to run_tests_worker().

(cherry picked from commit 012f5b9)

* bpo-34279, regrtest: Issue a warning if no tests have been executed (GH-10150)

(cherry picked from commit 9724348)

* test_regrtest: remove unused threading import
@miss-islington miss-islington mentioned this pull request Nov 29, 2018
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 29, 2018
@vstinner @miss-islington
[3.7] bpo-34279: Synchronize regrtest with master (pythonGH-10800) …
c02b89c 
* bpo-34605, libregrtest: Rename --slaveargs to --worker-args (pythonGH-9099)

Rename also run_tests_slave() to run_tests_worker().

(cherry picked from commit 012f5b9)

* bpo-34279, regrtest: Issue a warning if no tests have been executed (pythonGH-10150)

(cherry picked from commit 9724348)

* test_regrtest: remove unused threading import
(cherry picked from commit 8a73cac)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
miss-islington added a commit that referenced this pull request Nov 29, 2018
@miss-islington @vstinner
[3.7] bpo-34279: Synchronize regrtest with master (GH-10800) …
43d8126 
* bpo-34605, libregrtest: Rename --slaveargs to --worker-args (GH-9099)

Rename also run_tests_slave() to run_tests_worker().

(cherry picked from commit 012f5b9)

* bpo-34279, regrtest: Issue a warning if no tests have been executed (GH-10150)

(cherry picked from commit 9724348)

* test_regrtest: remove unused threading import
(cherry picked from commit 8a73cac)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
@pablogsal pablogsal deleted the no_test_run branch May 19, 2021 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

Assignees

@pablogsal pablogsal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

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