Validation tests

[mrunalp]

This is an alternative to #326 that uses go tests.

The tests can be invoked as
RUNTIME=runc make localvalidation

[mrunalp]

@Mashimiao @liangchenye @wking PTAL

[wking]

On Mon, Mar 06, 2017 at 02:25:00PM -0800, Mrunal Patel wrote: This is an alternative to #326 that uses go tests.

I think the concern about Go's builtin test framework was that you couldn't install an executable to run the tests [1]. You'd have to run runtime-validation tests from a source checkout. But I personally have no problem with requiring validation tests to be run from a source checkout [2], so whatever you find easiest to maintain works for me. [1]: #47 (comment) [2]: #61 (comment)

[Mashimiao]

We have merged #326, I think we can continue working on it

[mrunalp]

@hqhq @liangchenye @Mashimiao I think we should prefer this approach as it will be easier to write tests using go's testing support rather than building our own.

[Mashimiao]

Using Go's builtin test framework seems easier. But in my opinion, runtime-tools should be a tool set, runtime validation is one tool of it. Validating runtime by make seems not so formal.
@hqhq @liangchenye your opinions?

[liangchenye]

Hi @mrunalp , I prefer to working on #326. So we can release a tool and running it directory.
The Go buildin test framework is easier, but I think changing them into a regular function and calling them does not cost much.

+func TestValidateBasic(t *testing.T) {
}

[mrunalp]

@liangchenye @Mashimiao I think that rather than reworking this to use oci-runtime-tool runtime-validate, the approach in this PR is better as can directly access generate object and adjust it's properties. If we switch to command invocation then we will have to duplicate all options for generate to support test cases. A person executing the test case shouldn't care how this is implemented.

[wking]

On Fri, Mar 17, 2017 at 12:09:46PM -0700, Mrunal Patel wrote: I think that rather than reworking this to use oci-runtime-tool runtime-validate, the approach in this PR is better as can directly access generate object and adjust it's properties.

You can directly access the generate object with both approaches, can't you? Compare [1,2].

If we switch to command invocation then we will have to duplicate all options for generate to support test cases.

I think the difference between the two cases is that with this PR you can use ‘go test …’'s builtin harness, while with the approach in #326 you'd need to use an installable harness (like ginkgo). The decision probably comes down to “using the builtin harness is more convenient for runtime-tools maintainers” vs. “using an installed command is more convenient for runtime validators”, but I'm not clear enough on either of those to know which has more weight. [1]: https://github.com/opencontainers/runtime-tools/blob/ce55f9b813244c5054b3cccee882f594cacb9ca8/cmd/oci-runtime-tool/runtime_validate.go#L64 [2]: https://github.com/opencontainers/runtime-tools/pull/336/files#diff-b5e8ff579203d652582cf33ed2b39ad9R78

[mrunalp]

@wking I was talking of an approach where we modify 336 to invoke oci-runtime-tool runtime-validate test case.

[wking]

On Fri, Mar 17, 2017 at 01:37:54PM -0700, Mrunal Patel wrote: @wking I was talking of an approach where we modify 336 to invoke oci-runtime-tool runtime-validate test case.

Is that a thing? I think we want: * Something that the tester runs (e.g. ‘oci-runtime-tool runtime-validate’ or ‘go test ./...’). * A test harness (e.g. ginkgo or Go's builtin harness) * A test suite which that harness can drive. * A binary or two to be used in some of the suite's tests (e.g. busybox, runtimetest, …). If you're using a Go test harness, I think it would live inside runtime-validate. The test harness would not be calling runtime-validate as part of a single test (I agree that that would make accessing the generated config too awkward [1]). However, I'm not aware of any PRs to use ginko as a harness. #9 had a locally-implemented harness and this PR is using Go's builtin harness. If there's a possibility that someone will PR an installable harness into master soon (as @Mashimiao and @liangchenye seem to want [2,3]), then that would be great. If that PR is unlikely to materialize soon, I think we should land this one so we can move on. We can always translate tests between harnesses if we change our mind about harnesses later. [1]: #336 (comment) [2]: #336 (comment) [3]: #336 (comment)

[liangchenye]

After discussion with @mrunalp and @Mashimiao , I'm fine with this solution. We can focus on test targets and don't need to distract by the framework at present

[liangchenye]

LGTM

[Mashimiao]

LGTM