Skip to content
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

Prevent over stripping a drive letter in Windows #536

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kai2nenobu
Copy link

@kai2nenobu kai2nenobu commented Sep 25, 2019

This PR let use and deactivate command use an OS-specific path list separator instead of :.

This resolves #335.

About test

Existing tests fail in Windows.

Existing test result in windows on master (dcf0285)
> ver

Microsoft Windows [Version 10.0.18362.356]
> go version
go version go1.13 windows/amd64
> git rev-parse HEAD
dcf02850eddf464edb4c47495c7d1d79a9820832
> go test ./...
?       github.com/kai2nenobu/jabba     [no test files]
?       github.com/kai2nenobu/jabba/cfg [no test files]
--- FAIL: TestDeactivate (0.00s)
    deactivate_test.go:26: actual: [export PATH="/usr/local/bin:C:\Users\kai2nenobu\.jabba/jdk/zulu@1.8.72/bin:/system-jdk/bin:/usr/bin" export JAVA_HOME="/system-jdk" unset JAVA_HOME_BEFORE_JABBA] != expected: [export PATH="/usr/local/bin:/system-jdk/bin:/usr/bin" export JAVA_HOME="/system-jdk" unset JAVA_HOME_BEFORE_JABBA]
--- FAIL: TestUse (0.00s)
    use_test.go:47: actual: [export PATH="C:\Users\kai2nenobu\.jabba\jdk\1.7.2\bin;/usr/local/bin:C:\Users\kai2nenobu\.jabba/jdk/1.6.0/bin:/usr/bin" export JAVA_HOME="C:\Users\kai2nenobu\.jabba\jdk\1.7.2" export JAVA_HOME_BEFORE_JABBA="/system-jdk"] != expected: [export PATH="C:\Users\kai2nenobu\.jabba/jdk/1.7.2/bin:/usr/local/bin:/usr/bin" export JAVA_HOME="C:\Users\kai2nenobu\.jabba/jdk/1.7.2" export JAVA_HOME_BEFORE_JABBA="/system-jdk"]
FAIL
FAIL    github.com/kai2nenobu/jabba/command     0.606s
ok      github.com/kai2nenobu/jabba/command/fileiter    (cached)
ok      github.com/kai2nenobu/jabba/semver      (cached)
?       github.com/kai2nenobu/jabba/w32 [no test files]
FAIL

So I added new tests and let existing tests skip in Windows. However, I'm not familiar with golang testing, please review test code and let me know if there are smarter manners.

I tested this PR in travis ci on three OSs (linux, macos, windows) and two golang versions (1.9, 1.13). See travis ci log for more details.

…n windows

In windows, colon (:) is a separator for a drive letter. So
OS-specific path list separator (; in windows) should be used instead
of colon.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jabba deactivate cuts of drive letter
1 participant