Pin version of gojsonschema in tests#1682
Pin version of gojsonschema in tests#1682crosbymichael merged 1 commit intoopencontainers:masterfrom BooleanCat:master
Conversation
Signed-off-by: Will Martin <wmartin@pivotal.io>
| run bash -c "GOPATH='$GOPATH' go get github.com/xeipuuv/gojsonschema" | ||
| [ "$status" -eq 0 ] | ||
|
|
||
| run git -C "${GOPATH}/src/github.com/xeipuuv/gojsonschema" reset --hard 6637feb73ee44cd4640bb3def285c29774234c7f |
There was a problem hiding this comment.
Unit tests should not modify local environments like this. This will lose local changes.
There was a problem hiding this comment.
But gojsonschema isn't the thing under test, it's a dependency. The same pattern is already used to pin runtime-spec: https://github.com/opencontainers/runc/blob/master/tests/integration/spec.bats#L76
There was a problem hiding this comment.
This line makes two basically incorrect assumptions that will break:
- An alternative version of gojsonschema may be in use that could fix this. We can never test with this.
- A
GOPATHmay consist of multiple paths.
Just because the same pattern is in use doesn't mean that it is correct.
|
LGTM |
|
Can we merge as a temporary stopgap to unblock other PRs? Many other repo added a workaround like this for now, we can create an issue to revert and fully fix this once we know what to do. @mrunalp ptal |
|
LGTM A fix after this should be moving these out of the bats files. It makes no sense that deps are scattered and hidden in various files. ( @BooleanCat not your fault, its good that you kept it consistent for now ) |
Fixes #1680 (partially). This is suggested as a temporary fix to runc so that CI can go green and development can continue.
We will detail a potential problem we've found with opencontainers json schema validation in a email to the mailing list.
Signed-off-by: Will Martin wmartin@pivotal.io