Add "--deselect" command line option by uSpike · Pull Request #3201 · pytest-dev/pytest

uSpike · 2018-02-09T23:50:29Z

I implemented this as a new CLI option --deselect since it does have different functionality than --ignore, which completely ignores the file.

The code matches any nodeid that startswith() any --deselect flag.

Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
Target the features branch for new features and removals/deprecations.
Include documentation when adding new features.
Include new tests or update existing tests when applicable.
Add yourself to AUTHORS in alphabetical order

coveralls · 2018-02-10T00:33:42Z

Coverage increased (+0.01%) to 92.65% when pulling 774c539 on uSpike:deselect_cli into 063e2da on pytest-dev:features.

nicoddemus

Thanks @uSpike!

nicoddemus · 2018-02-15T21:23:11Z

+    """)
+    result = testdir.runpytest("--deselect=test_a.py::test_a2[1]")
+    assert result.ret == 0
+    result.stdout.fnmatch_lines(["*11 passed, 1 deselected*"])


This should be 10 passed, 1 deselected right?

Also I think it would be better to use -v and deselect two tests and ensure they are not being run. You can parametrize test_a2 using a smaller value (say 3) so the test remains tidy and small.

nicoddemus · 2018-02-15T21:23:56Z

+    for colitem in items:
+        for opt in deselectopt:
+            if colitem.nodeid.startswith(opt):
+                deselected.append(colitem)


I believe this needs a break after deselected.append(colitem)

RonnyPfannschmidt · 2018-02-15T21:44:45Z

+    deselected = []
+    for colitem in items:
+        for opt in deselectopt:
+            if colitem.nodeid.startswith(opt):


startswith can take a tuple which can be used as a much better selector

instead of a loop and the for else hack it could simply use colitem.nodeid.startswith(deselect_prefixes)

i presume the break was not added due to the expectation of the prefixes being distinct/non-overlapping

Oh cool, thanks I didnt realize str.startswith() takes a tuple. That's great.

RonnyPfannschmidt · 2018-02-15T21:45:45Z

                    help="try to interpret all arguments as python packages.")
    group.addoption("--ignore", action="append", metavar="path",
                    help="ignore path during collection (multi-allowed).")
+    group.addoption("--deselect", action="append", metavar="item",


i propose a metavar of nodeid_prefix

uSpike · 2018-02-15T22:05:49Z

@RonnyPfannschmidt @nicoddemus I've implemented your suggestions, thanks much for the feedback.

RonnyPfannschmidt · 2018-02-16T08:08:17Z

+    remaining = []
+    deselected = []
+    for colitem in items:
+        if colitem.nodeid.startswith(tuple(deselect_prefixes)):


well done, please also move the call to tuple to the place where deselect_prefixes is declared

Fixes pytest-dev#3198

blueyed · 2018-02-16T14:35:26Z

In #3198 you said:

I propose to accept test ids with --ignore:

This would not need a new option then.

btw: I've also thought about this being something negating -k (e.g. -K) - but that would be a different feature (since it uses different logic / evaluation).

uSpike · 2018-02-16T15:25:35Z

@blueyed yes I had originally thought that I could add to --ignore but the functionality of --ignore ended up being fundamentally different than what I needed. I will update #3198.

nicoddemus · 2018-02-16T22:50:40Z

Thanks @uSpike!

blueyed · 2018-02-19T22:23:06Z

I still think that it is confusing to have --ignore and --deselect now, and that support for foo.py::X should get added to --ignore instead.

--deselect requires the filename as a prefix already, and therefore is only a specialized version of --ignore, isn't it?

twmr · 2018-02-20T08:28:47Z

I guess that it makes sense to mentioned in the docs what the differences between --ignore and --deselect are

uSpike · 2018-02-20T16:39:22Z

--ignore is not exactly the same as --deselect. Paths that are excluded by --ignore are not reported anywhere via pytest_deselected or on the CLI. Items that are deselected by --deselect are passed to pytest_deselected and reported as so on the CLI.

I'm not sure if it's more or less confusing with --ignore if things that look like paths are not considered for collection at all, and things that look like node ids are deselected. It's a simple implementation and I'd be happy to submit another PR if there's interest.

uSpike · 2018-02-20T16:45:55Z

I have a working implementation here: uSpike@8b98792

blueyed · 2020-02-19T20:27:35Z

@uSpike
Thanks for following up with merging this with --ignore, and sorry for having missed it previously.
Would you like to follow up on that? (given that there just was confusion with --deselect not handling paths)

uSpike force-pushed the deselect_cli branch from 52a4c2f to d320da3 Compare February 12, 2018 17:43

nicoddemus requested changes Feb 15, 2018

View reviewed changes

RonnyPfannschmidt reviewed Feb 15, 2018

View reviewed changes

uSpike force-pushed the deselect_cli branch 2 times, most recently from d36ada9 to f7828a8 Compare February 15, 2018 22:03

uSpike force-pushed the deselect_cli branch from f7828a8 to 5f0c395 Compare February 15, 2018 22:08

RonnyPfannschmidt reviewed Feb 16, 2018

View reviewed changes

uSpike force-pushed the deselect_cli branch 2 times, most recently from 7386d9c to 0e2522c Compare February 16, 2018 14:15

Add --deselect command line option

774c539

Fixes pytest-dev#3198

uSpike force-pushed the deselect_cli branch from 0e2522c to 774c539 Compare February 16, 2018 14:17

uSpike mentioned this pull request Feb 16, 2018

Ignore individual tests #3198

Closed

nicoddemus approved these changes Feb 16, 2018

View reviewed changes

nicoddemus merged commit b1abe5d into pytest-dev:features Feb 16, 2018

uSpike deleted the deselect_cli branch February 20, 2018 16:23

omnilinguist mentioned this pull request Oct 12, 2018

--deselect does not work with tests inside classes #4127

Closed

blueyed mentioned this pull request Feb 19, 2020

--deselect doesn't work with absolute paths #6751

Open

4 tasks

Uh oh!

Conversation

uSpike commented Feb 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Feb 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

nicoddemus Feb 15, 2018

Choose a reason for hiding this comment

Uh oh!

nicoddemus Feb 15, 2018

Choose a reason for hiding this comment

Uh oh!

nicoddemus Feb 15, 2018

Choose a reason for hiding this comment

Uh oh!

RonnyPfannschmidt Feb 15, 2018

Choose a reason for hiding this comment

Uh oh!

uSpike Feb 15, 2018

Choose a reason for hiding this comment

Uh oh!

RonnyPfannschmidt Feb 15, 2018

Choose a reason for hiding this comment

Uh oh!

uSpike commented Feb 15, 2018

Uh oh!

RonnyPfannschmidt Feb 16, 2018

Choose a reason for hiding this comment

Uh oh!

blueyed commented Feb 16, 2018

Uh oh!

uSpike commented Feb 16, 2018

Uh oh!

nicoddemus commented Feb 16, 2018

Uh oh!

blueyed commented Feb 19, 2018

Uh oh!

twmr commented Feb 20, 2018

Uh oh!

uSpike commented Feb 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uSpike commented Feb 20, 2018

Uh oh!

blueyed commented Feb 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

uSpike commented Feb 9, 2018 •

edited

Loading

coveralls commented Feb 10, 2018 •

edited

Loading

uSpike commented Feb 20, 2018 •

edited

Loading