Prevent branch mode warnings in unstaged mode #183
Conversation
There was a problem hiding this comment.
Hi @stevelacey! Thanks a lot for your contribution 🎉
It is funny that now that I see this part of the code, I realize that an if/else code has avoided this issue. But back then, I assumed that more modes would come in fast. It's too early for that. haha
I've left a comment for you. Could you please add a regression test to prevent this issue from happening again and add a line to our CHANGELOG? 🙏🏽
I appreciate your contribution.
pytest_picked/plugin.py
Outdated
| @@ -53,8 +53,7 @@ def _get_affected_paths(config): | |||
| _write(config, [error]) | |||
| config.args = [] | |||
| raise ValueError(error) | |||
| else: | |||
| return mode.affected_tests() | |||
| return mode[0](*mode[1:]).affected_tests() | |||
There was a problem hiding this comment.
What about changing it to something more readable?
modes = {
"branch": {
"class": Branch,
"args": (test_file_convention, parent_branch)
},
"unstaged": {
"class": Unstaged,
"args": (test_file_convention)
},
}
try:
mode = modes[picked_mode]
except KeyError:
error = "Invalid mode. Options: `{}`.".format(", ".join(modes.keys()))
_write(config, [error])
config.args = []
raise ValueError(error)
return mode["class"](*mode["args"]).affected_tests()
It is more verbose, but it helps when reading the code.
There was a problem hiding this comment.
I also had a version briefly where I used partial to pass Branch its extra kwarg, if you'd prefer that.
There was a problem hiding this comment.
Alternatively, maybe lets pass test_file_convention explicitly, and then pass all options as kwargs to all modes and it's up to the mode to make use of whichever things it supports?
|
Hi @stevelacey, you closed the PR. Is that alright? |
|
Oh, that's funny, I wrote |
|
@anapaulagomes hows that look? LGTM? 🙏 |
|
Thank you @stevelacey! Almost there! I appreciate it if you could write a test and add a line to the CHANGELOG. :) |
stevelacey
changed the title
|
@anapaulagomes done |
stevelacey
changed the title
|
@anapaulagomes great, thanks 🙌 can you release 0.5.1 on PyPi? 🙏 |
|
It's on my to-do list. The current process is not working because some parts are deprecated, so I'll have to make some changes or do it manually (and my current machine is not ready for it). Bear with me :) |
|
There you go @stevelacey https://pypi.org/project/pytest-picked/0.5.1/ |
Feel free to suggest a different way to shake this, I went for the quickest way to avoid the extra init.
Fixes #182