-
-
Notifications
You must be signed in to change notification settings - Fork 528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add tests for CliEnv #3204
Add tests for CliEnv #3204
Conversation
371cb60
to
eabf094
Compare
eabf094
to
330b4db
Compare
All but one of the tests in this test module are actually testing env_select functionality through calling other code that in turn uses env_select (CliEnv, EnvSelector and register_env_select_flags). We move that one test to the top of the file, but we do not indicate the boundary between the groups (or even explain that the grouping exists) at the request of @gaborbernat.
Some of the behaviour here, such as treatment of zero-length environment names specified by the user, is not really intentional, but apparently an artifact of the method used to parse the environment name list supplied by the user.
I find this makes the tests significantly less readable, so I'm commiting this separately so we can decide what to do about it. The options are: 1. Squash this with the previous, just using the auto-enforced formatting. 2. Leave these commits as-is, so we at least can go back to the more readable original formatting. 3. Do whatever needs to be done to keep the auto-reformatter from touching that particular bit of code. I have a preference for 3, but won't argue any other decision.
330b4db
to
c53fefb
Compare
I'm not clear on why this PR is still saying that there are changes requested. I clicked "See review" and it just takes me to requests that are from before I pushed up the fixes and that are marked outdated and resolved. Can someone explain to me what's happening here? |
Until I approve, will stay in that status. That's how GitHub works. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok! I've seen something else similar; GitHub keeps saying things that make me wonder if I missed fixing something. But I'll just stop worrying about it. Please do poke me if you see a PR where I've responded to things but there are still things that I need to fix, thoug. |
This is a start on issue #3199. The plan for that involves changing CliEnv; this starts documenting its current behaviour by adding tests for it.
The final reformatting commit may need to be squashed into the previous commit; see that commit message for details.
No news fragment because this isn't a user-visible change. No documentation yet because that will be informed by review of this commit.
tox -e fix
)docs/changelog
folder