Fix config read+support tests on Windows

[NathanDeMaria]

Description

quilt3 fails to load config on Windows as of 3.1.12, giving me this error on import quilt3:

AttributeError: 'WindowsPath' object has no attribute 'read'

from inside the yaml code. Tracing around, I believe the root is that this check should accept all pathlib.Paths, not just pathlib.PosixPath. Without that check, quilt3 passes a pathlib object instead of a stream into yaml.safe_load, which triggers the above error.

Changing this worked for me locally.

[akarve]

@NathanDeMaria Nice catch. Thank you. Do you feel comfortable modifying the .circleci/config.yml file to test windows in this PR? That would be a great help and we could extend official support.

[codecov-io]

Codecov Report

Merging #1565 into master will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1565      +/-   ##
==========================================
+ Coverage   84.68%   84.75%   +0.07%     
==========================================
  Files          27       27              
  Lines        3897     3897              
==========================================
+ Hits         3300     3303       +3     
+ Misses        597      594       -3     

Impacted Files	Coverage Δ
quilt3/search_util.py	75.86% <0.00%> (-3.45%)	⬇️
quilt3/util.py	81.58% <0.00%> (+1.44%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 894c98c...feb5318. Read the comment docs.

[NathanDeMaria]

@akarve does that look reasonable? I'm not very familiar with CircleCI, but this seems to be working

[sir-sigurd]

The job is successful, but actually the tests fail (see the job log). I didn't check thoroughly why it happens, but might make sense to try set bash as shell (https://circleci.com/docs/2.0/hello-world-windows/#specifying-a-shell-with-the-windows-executor).

[akarve]

@NathanDeMaria Good progress. I took a quick look at the failures and we might be down to actual platform differences at this point (e.g. around \r). If you bump into any tricky issues, just ask and we'll help.

In theory the unit tests should run locally on a Windows box as well.

[NathanDeMaria]

I have a question on what the intended behavior should be for the from_path function - on Windows, that makes the PhysicalKey have /'s instead of \\, but tests like this one expect that key to be the same as the string form of a Windows path (with \\s)

From context, it looks like we do want the PhysicalKey to have /s (because it's an AWS S3 key), so that test_install_subpackage test (and a few others like it) should be expecting something like PhysicalKey.from_path(path).path (to avoid repeating the .from_path logic). Does that fit with what you're looking for?

[NathanDeMaria]

These tests pass on a clean checkout on Windows, but fail in the CI builds. My best guess is CircleCI is using an old checked out version of the .txt files (still with Windows line endings). I'm not sure how to (or if I have permission to) start the builds from a clean checkout, but I think that's all the work that remains here

[sir-sigurd]

These tests pass on a clean checkout on Windows, but fail in the CI builds. My best guess is CircleCI is using an old checked out version of the .txt files (still with Windows line endings). I'm not sure how to (or if I have permission to) start the builds from a clean checkout, but I think that's all the work that remains here

CircleCI runs at first git clone and then git checkout and as I understand git fixes newlines only when it creates files.
Anyway I decided to use different approach to avoid .gitattributes files:

quilt/.circleci/config.yml

Line 36 in feb5318

- run: git config --global core.autocrlf false # for Windows

Do my changes look OK for you?

[NathanDeMaria]

Without the .gitattributes, tests will fail locally for new Windows checkouts, unless people have that git config set. It'll save people a headache to keep around .gitattributes (see tests are failing, realize it's because they need a certain git config set, make the change, 🔁 reclone), but I'm not sure how many people plan on running theses tests locally in Windows, so maybe it's not a big deal 😆

[sir-sigurd]

Without the .gitattributes, tests will fail locally for new Windows checkouts, unless people have that git config set. It'll save people a headache to keep around .gitattributes (see tests are failing, realize it's because they need a certain git config set, make the change, repeat reclone), but I'm not sure how many people plan on running theses tests locally in Windows, so maybe it's not a big deal laughing

Makes sense. Added .gitattributes in 9c4362d.

[sir-sigurd]

@NathanDeMaria, thanks for your contribution!