add CheckSemVer, CheckRootfsPath, CheckPlatform unit test #273
Conversation
| - $HOME/gopath/bin/golint ./... | ||
| - $HOME/gopath/bin/git-validation -run DCO,short-subject -v -range ${TRAVIS_COMMIT_RANGE} | ||
| - go test -v ./... | ||
| - make |
There was a problem hiding this comment.
Probably better to drop the explicit vet and golint calls and have:
script:
- $HOME/gopath/bin/git-validation -run DCO,short-subject -v -range ${TRAVIS_COMMIT_RANGE}
- make all test
There was a problem hiding this comment.
how about test first then try to compile?
- make test
- make all
| $(RUNTIME_TOOLS_LINK): | ||
| ln -sf $(CURDIR) $(RUNTIME_TOOLS_LINK) | ||
|
|
||
| .PHONY: test .gofmt .govet .golint |
There was a problem hiding this comment.
You need to add .gotest here as well.
There was a problem hiding this comment.
Still missing a .PHONY entry for .gotest?
validate/validate_test.go
Outdated
| rootfsDir := "rootfs" | ||
| rootfsNonDir := "rootfsfile" | ||
| rootfsNonExists := "rootfsnil" | ||
| os.MkdirAll(filepath.Join(tmpBundle, rootfsDir), 0750) |
There was a problem hiding this comment.
This is a test run entirely by one user, so you can use 0700 if you want to restrict permissions. Alternatively, you can use 0777 and rely on the caller's umask. But specifying 0750 seems like a strange middle ground ;).
There was a problem hiding this comment.
I just used to set 0750 to all the created dir. I don't mind to change it to 0700. Update comes soon.
| expected bool | ||
| }{ | ||
| {rspec.Version, true}, | ||
| //FIXME: validate currently only handles rpsec.Version |
There was a problem hiding this comment.
CheckSemVer already has separate checks for “valid SemVer” and “recognized SemVer”. What we're missing is support for past versions of the spec, and that problem is more general than this single test.
There was a problem hiding this comment.
We don't have the 'official supported version', so I prefer to keep this and update this once the problem of supporting past version is fixed.
There was a problem hiding this comment.
I prefer to keep this and update this once the problem of supporting past version is fixed.
That's fine, although it seems like a repo-wide issue that would be better served by a GitHub issue ;).
liangchenye
force-pushed
the
unittest
branch
from
d6da700 to
45f2558
Compare
|
LGTM |
validate/validate_test.go
Outdated
| rootfsDir := "rootfs" | ||
| rootfsNonDir := "rootfsfile" | ||
| rootfsNonExists := "rootfsnil" | ||
| os.MkdirAll(filepath.Join(tmpBundle, rootfsDir), 0700) |
validate/validate_test.go
Outdated
| rootfsNonDir := "rootfsfile" | ||
| rootfsNonExists := "rootfsnil" | ||
| os.MkdirAll(filepath.Join(tmpBundle, rootfsDir), 0700) | ||
| os.Create(filepath.Join(tmpBundle, rootfsNonDir)) |
liangchenye
force-pushed
the
unittest
branch
from
45f2558 to
2ff2484
Compare
|
rebased, @mrunalp PTAL |
|
LGTM |
|
need rebase |
|
rebased, PTAL @Mashimiao @hqhq @mrunalp |
|
failed with CI |
liangchenye
force-pushed
the
unittest
branch
2 times, most recently
from
e6b4918 to
176de83
Compare
|
updated @Mashimiao |
|
LGTM |
validate/validate_test.go
Outdated
| } | ||
| } | ||
|
|
||
| func TestCheckPlatform(t *testing.T) { |
There was a problem hiding this comment.
Platform is already removed from runtime-spec, so we don't need to add this is we're aiming at 1.0.
Signed-off-by: liangchenye <[email protected]>
|
LGTM |
1 similar comment
|
LGTM |
5 participants
Signed-off-by: liangchenye [email protected]