Refactor implementation of ParseUlimit function

docker / go-units

Parse and print size and time units in human-readable format

https://godoc.org/github.com/docker/go-units

Apache License 2.0

218 stars 38 forks source link

Refactor implementation of ParseUlimit function #6

Closed kezhuw closed 8 years ago

kezhuw commented 8 years ago

Two problems exist in old code:

The condition in statement if len(limitVals) > 2 never evaluates to true, because of strings.SplitN(s, sep string, n int) will return at most n substrings when n > 0.
Error err is ignored in parsing hard limitation value. Thus we may get inaccurate error message when invalid hard limitation value encountered.

Signed-off-by: Kezhu Wang kezhuw@gmail.com

calavera commented 8 years ago

can we have some tests to make sure this is always correct?

calavera commented 8 years ago

the change LGTM but I'd like to be sure that we cover all cases with our tests.

kezhuw commented 8 years ago

New test is added in commit f21b2d4 to break old code. Seems that no ci build happened for that commit. I paste error message from my local environment:

➜  go-units git:(f21b2d4) go test
--- FAIL: TestParseUlimitInvalidValueType (0.00s)
    ulimit_test.go:65: expected error on bad value type, but got `ulimit soft limit must be less than or equal to hard limit: 1024 > 0`
FAIL
exit status 1
FAIL    github.com/docker/go-units  0.007s

Commit d4a9b96 passes this test as the ci build shows.

calavera commented 8 years ago

Thank you, merging!