Add Windows support #20
Conversation
cagataygurturk
commented
Jan 21, 2025
•
cagataygurturk
commented
- Fixes Support Windows _netrc instead of .netrc #14
- Switch from path to path/filepath for multi-platform line separators
- Fix the AddConfigs test for Windows by removing the new lines in the test data. Different new line characters in Windows were causing tests always to fail
| {"test-key": "test-value"} |
There was a problem hiding this comment.
Any chance we can revert this change? This is unrelated to the fix and the original key.json wasn't incorrectly formatted either.
There was a problem hiding this comment.
It is actually related to the fix. Without this change, tests in Windows are not passing due to new line differences. It was a quick fix to mitigate that. Should I revert and leave the tests broken in Windows or leave like this?
There was a problem hiding this comment.
Can you explain more? I thought this file was only used in the add the first location with json key test case?
There was a problem hiding this comment.
Oh I see what you mean. Is it because \n and \r\n leading to different base64 encoding of json key? If so, this change LGTM
There was a problem hiding this comment.
Yes! It took me quite some time to understand why tests were passing on Mac/Linux and not on Windows, then I figured out that was the reason. And I just got rid of newlines since they are not too related to what we are trying to test.
|
Thanks @yihanzhen for the review. Can we merge and release if all looks ok? The issues solved here are quite annoying for Windows users and even Google Cloud documentation does not mention about the possible workarounds. |
