User expansion #352
User expansion #352
Conversation
tleonhardt
left a comment
tleonhardt
left a comment
There was a problem hiding this comment.
Looks good.
Consider dealing with the following unused local variables in the _print_topcis() method:
- help_topics
- doc
|
|
||
| users = [] | ||
|
|
||
| # Windows lacks the pwd module so we can't get a list of users. |
tleonhardt
left a comment
tleonhardt
left a comment
There was a problem hiding this comment.
Fix failing unit tests on AppVeyor.
Your code and unit tests look fine to me and pass on my Windows VM.
Potentially just skip the one failing test if you can't find a workaround.
tests/test_completion.py
Outdated
| assert cmd2_app.path_complete(text, line, begidx, endidx) == [] | ||
| def test_path_completion_expand_user_dir(cmd2_app): | ||
| # Get the current user | ||
| user = getpass.getuser() |
There was a problem hiding this comment.
For some reason this is causing a unit test failure on the AppVeyor Windows CI build/test systems. All unit tests pass for this branch on my Windows 10 VM for both Python 3.6 and 2.7.
But for some reason this is hosed on AppVeyor. As far as I can tell, the problem is with AppVeyor and not with the code.
If you can't find a good workaround quickly, I recommend just skipping this test on Windows.
There was a problem hiding this comment.
For some reason getpass.getuser() fails when running on AppVeyor. The user must not have any of these environment variables. Unfortunately the fallback imports the pwd module which doesn't exist on Windows.
for name in ('LOGNAME', 'USER', 'LNAME', 'USERNAME'):
user = os.environ.get(name)
if user:
return user
# If this fails, the exception will "explain" why
import pwd
return pwd.getpwuid(os.getuid())[0]I am trying a workaround.
Codecov Report
@@ Coverage Diff @@
## master #352 +/- ##
==========================================
+ Coverage 89.64% 89.75% +0.11%
==========================================
Files 1 1
Lines 2008 2021 +13
==========================================
+ Hits 1800 1814 +14
+ Misses 208 207 -1
Continue to review full report at Codecov.
|
tleonhardt
left a comment
tleonhardt
left a comment
There was a problem hiding this comment.
Good to go as long as all tests pass.
Consider talking with Eric about cleaning up the unused local variables ...
| def test_path_completion_expand_user_dir(cmd2_app): | ||
| # Get the current user. We can't use getpass.getuser() since | ||
| # that doesn't work when running these tests on Windows in AppVeyor. | ||
| user = os.path.basename(os.path.expanduser('~')) |
There was a problem hiding this comment.
Assuming the unit test all pass, this looks OK. They still pass on my system.
Allowing the use of ~user tab completion in paths