Contributor
Path('R:\repo').resolve() seems to resolve to `\computername\repos\repo' when using a mounted drive. The path in a form of R:\repo\ is technically an absolute path already so it doesn't need converting.
Closed
@erinxocon Thank you very much for your help Erin! I'm trying to figure out how to pull your changes and rerun my tests to verify that it resolves my original issue.
Contributor Author
@ahendry2688 Just pull from master and then checkout the unc-paths branch. I do see an error in CI, but think it might just be transient as it didn't fail the other windows test.
@erinxocon Do you mean the following?:
git pull origin master, then
git checkout -b unc-paths.
Contributor Author
@ahendry2688 indeed. Then ensure pipenv is uninstalled and set it up as a development dependency by doing pip install -e .
@erinxocon Thanks for your help. I'm still getting errors. Here are the commands I issued:
pip uninstall pipenv
pip install pytest-xdist
git pull origin master
git checkout -b unc-paths
pip install -e .
pipenv run pytest tests -n 24
And here is the output (see attached .txt file):
(EDIT: I realized I'm running on a different machine. I should have used -n auto above. I'll rerun test and give you output in another comment)
Member
All you should need to run is set PYPI_VENDOR_DIR=".\tests\pypi\" && set PYTHONIOENCODING="utf-8" && cd pipenv && pip install -e . && pipenv install --dev && pipenv run pytest -v -n 4 --ignore=".\\pipenv\\patched" --ignore=".\\pipenv\\vendor" tests
@erinxocon Even with techalchemy's suggestions, my tests still report errors. I will continue to look into this. Please let me know if you do not want my text file outputs anymore and I will stop sending them.
Contributor Author
@techalchemy pipfile checks if the file is exists and if not it returns none, so it handles checking if the files exists and it's caught by normalize if it's none. Probably don't need to use resolve as pipfile will always pass it an absolute path, but if someone sets the env variable to change the pipfile location I suppose resolve is necessary so I'll keep the PR as it is.
Member
@erinxocon there was a concern around it accurately writing to a Pipfile if the pipfile itself is stored on a share, I don't currently have a way to test this scenario but I suspect if we aren't calling normalize_path everywhere it might be a source of problems. We might need to brainstorm a unit test for an edge case or something
Contributor Author
@techalchemy so far this is the only way I can get it to write to the pipfile right now with my mounted drives. Do you know how to mount shares on app veyor? I'll have to do some research to see about setting up a test like that
Thank you both so much for all your hard work! If it's any consolation, my company uses all shared drives to collaborate with colleagues as we have yet to purchase a private GitHub repo. This PR would be a huge help to us, and I'm hoping to other users as well. Thank you!
Contributor Author
I ran the test suite from a mounted drive and got no errors with the fix I put in place. Thought of a cleanup idea, but this appears to be working. Now for a test case on app veyor..
Member
@erinxocon when you get a chance can you uncomment the appveyor matrix?
Contributor Author
@techalchemy done, sorry bout that, was messing around with app veyor. Squashed the commits out of existence.
| if loc.is_absolute(): | ||
| return normalize_drive(str(loc)) | ||
| else: | ||
| return normalize_drive(str(loc.resolve())) |
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on our findings in the other PR we've been hammering away at, can you make this second part into:
else: try: loc = loc.resolve() except OSError: loc = loc.absolute() return normalize_drive(str(loc))
In order to handle Ramdisks...
Contributor Author
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done