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

fix unit test of influxd in the windows #5489

Merged
merged 3 commits into from
Feb 5, 2016

Conversation

mei-rune
Copy link
Contributor

fix unit test of influxd in the windows

@@ -360,7 +360,10 @@ Options:
func retentionAndShardFromPath(path string) (retention, shard string, err error) {
a := strings.Split(path, "/")
if len(a) != 3 {
return "", "", fmt.Errorf("expected database, retention policy, and shard id in path: %s", path)
a = strings.Split(path, "\\")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unfamiliar with what the contents of the path would be, but would os.PathSeparator be a better method and do something like:

a := srings.Split(path, os.PathSeparator)

If not, then we may want to make a specific windows/unix file for this specific function, as it feels the issue is platform specific.

@corylanou
Copy link
Contributor

Looks good. One question to be addressed. Thanks!

@mei-rune mei-rune force-pushed the cmd_influxd branch 2 times, most recently from 60419e2 to 91f47ab Compare February 3, 2016 05:11
@jwilder jwilder modified the milestone: 0.11.0 Feb 3, 2016
return net.JoinHostPort(DefaultHostname, port)
address, err := DefaultHost(DefaultHostname, addr)
if nil != err {
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous behavior was to return addr here. It should not panic.

fix rename file fail on the windows
@mei-rune
Copy link
Contributor Author

mei-rune commented Feb 5, 2016

all unit test will ok after this patch is merged.
I hope merge it as soon as possible. I have a holiday soon.

jwilder added a commit that referenced this pull request Feb 5, 2016
fix unit test of influxd in the windows
@jwilder jwilder merged commit 6b3e18b into influxdata:master Feb 5, 2016
@jwilder
Copy link
Contributor

jwilder commented Feb 5, 2016

Thanks @runner-mei!

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.

3 participants