TST: check_comprehensiveness compat for --lf and -k #37402
Conversation
| for combo in combos: | ||
| if not has_test(combo): | ||
| raise AssertionError(f"test method is not defined: {cls.__name__}, {combo}") | ||
| opts = request.config.option |
There was a problem hiding this comment.
why not just parameterize on this?
There was a problem hiding this comment.
huh?
this is checking if pytest is invoked with --lf or -k foo
There was a problem hiding this comment.
sure, we don't do this anywhere else, so why is this actually needed here?
There was a problem hiding this comment.
#23930 this will produce unhelpful errors when running only a subset of tests in this file
There was a problem hiding this comment.
i saw the issue, my question is that this is either a very specific fix, in which case it might be ok, or something that actually is generally a problem. I would suggest that we could refactor the tests to simply avoid having to do this, rather than trying to address this very specifically.
There was a problem hiding this comment.
big-picture, i think check_comprehensiveness is a good idea and we should eventually hope to use something like it in more contexts.
even if we refactored the tests in this file (by no means a small feat), if we want to have a check for comprehensiveness it would still look something like this function
There was a problem hiding this comment.
ok fair, just don't want this to be specific here, e.g. it can easily get lost
|
cc @TomAugspurger is this roughly what you had in mind? |
|
Yeah, this seems nice. I want both of
so I think this looks about ideal. |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff