Skip to content

Commit

Permalink
cmd/runtimetest/main: Use TAP diagnostics for errors
Browse files Browse the repository at this point in the history
This lets us print SHOULD violations even if they're non-fatal.  It
also gets us back to consumable TAP output.  I added a /foo entry to
defaultFS for testing, and before this commit:

  $ ./runtimetest
  TAP version 13
  not ok 1 - root filesystem
  ok 2 - hostname
  not ok 3 - process
  not ok 4 - mounts
  not ok 5 - user
  ok 6 - rlimits
  ok 7 - capabilities
  ok 8 - default symlinks
  ok 9 - default file system
  ok 10 - default devices
  ok 11 - linux devices
  not ok 12 - linux process
  ok 13 - masked paths
  ok 14 - oom score adj
  ok 15 - read only paths
  ok 16 - rootfs propagation
  ok 17 - sysctls
  ok 18 - uid mappings
  ok 19 - gid mappings
  1..19
  6 errors occurred:

  * rootfs must not be readonly
  Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#root
  * Cwd expected: /, actual: /home/wking/.local/lib/go/src/github.com/opencontainers/runtime-tools
  * mounts[1] {/tmp tmpfs none []} does not exist
  Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#mounts
  * mounts[2] {/dev devtmpfs devtmpfs []} mounted before mounts[0] {/tmp tmpfs none []}
  Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#mounts
  * UID expected: 1, actual: 1000
  * Process arguments expected: ./runtimetest, actual: sh
  $ echo $?
  1

The TAP 13 spec doesn't cover script exit-code handling [1], but that
exit code confuses prove unless you set --ignore-exit:

  $ prove ./runtimetest
  ./runtimetest .. 1/? 6 errors occurred:

  * rootfs must not be readonly
  Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#root
  * Cwd expected: /, actual: /home/wking/.local/lib/go/src/github.com/opencontainers/runtime-tools
  * mounts[1] {/tmp tmpfs none []} does not exist
  Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#mounts
  * mounts[2] {/dev devtmpfs devtmpfs []} mounted before mounts[0] {/tmp tmpfs none []}
  Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#mounts
  * UID expected: 1, actual: 1000
  * Process arguments expected: ./runtimetest, actual: sh
  ./runtimetest .. Dubious, test returned 1 (wstat 256, 0x100)
  Failed 5/19 subtests

  Test Summary Report
  -------------------
  ./runtimetest (Wstat: 256 Tests: 19 Failed: 5)
    Failed tests:  1, 3-5, 12
    Non-zero exit status: 1
  Files=1, Tests=19,  0 wallclock secs ( 0.02 usr +  0.00 sys =  0.02 CPU)
  Result: FAIL

Note the "Dubious, test returned 1".  And with the diagnostics written
to stderr, there's no way to have prove catch that.

With this commit, we're back to TAP-compatible output and exit codes,
using TAP's diagnostics [2] to share the failure details.

  $ ./runtimetest
  TAP version 13
  not ok 1 - root filesystem
  # rootfs must not be readonly
  # Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#root
  ok 2 - hostname
  not ok 3 - process
  # Cwd expected: /, actual: /home/wking/.local/lib/go/src/github.com/opencontainers/runtime-tools
  not ok 4 - mounts
  # mounts[1] {/tmp tmpfs none []} does not exist
  # Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#mounts
  not ok 5 - mounts
  # mounts[2] {/dev devtmpfs devtmpfs []} mounted before mounts[0] {/tmp tmpfs none []}
  # Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#mounts
  not ok 6 - user
  # UID expected: 1, actual: 1000
  ok 7 - rlimits
  ok 8 - capabilities
  ok 9 - default symlinks
  ok 10 - default file system
  ok 11 - default devices
  ok 12 - linux devices
  not ok 13 - linux process
  # Process arguments expected: ./runtimetest, actual: sh
  ok 14 - masked paths
  ok 15 - oom score adj
  ok 16 - read only paths
  ok 17 - rootfs propagation
  ok 18 - sysctls
  ok 19 - uid mappings
  ok 20 - gid mappings
  1..20
  $ echo $?
  0

You can use prove without --ignore-edit to summarize:

  $ prove ./runtimetest
  ./runtimetest .. Failed 6/20 subtests

  Test Summary Report
  -------------------
  ./runtimetest (Wstat: 0 Tests: 20 Failed: 6)
    Failed tests:  1, 3-6, 13
  Files=1, Tests=20,  0 wallclock secs ( 0.03 usr +  0.00 sys =  0.03 CPU)
  Result: FAIL

And prove knows that the tests failed from TAP, with the script exit
code saying "we were able to run the tests" and not speaking to
pass/fail:

  $ echo $?
  1

If that's too compact, you can ask prove to show failures and
comments:

  $ prove -fo ./runtimetest
  ./runtimetest .. 1/?
  not ok 1 - root filesystem
  # rootfs must not be readonly
  # Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#root
  not ok 3 - process
  # Cwd expected: /, actual: /home/wking/.local/lib/go/src/github.com/opencontainers/runtime-tools
  not ok 4 - mounts
  # mounts[1] {/tmp tmpfs none []} does not exist
  # Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#mounts
  not ok 5 - mounts
  # mounts[2] {/dev devtmpfs devtmpfs []} mounted before mounts[0] {/tmp tmpfs none []}
  # Refer to: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#mounts
  not ok 6 - user
  # UID expected: 1, actual: 1000
  not ok 13 - linux process
  # Process arguments expected: ./runtimetest, actual: sh
  ./runtimetest .. Failed 6/20 subtests

  Test Summary Report
  -------------------
  ./runtimetest (Wstat: 0 Tests: 20 Failed: 6)
    Failed tests:  1, 3-6, 13
  Files=1, Tests=20,  0 wallclock secs ( 0.03 usr +  0.00 sys =  0.03 CPU)
  Result: FAIL

You can also turn that SHOULD error into a failure by cranking up the
compliance level:

  $ prove -fo ./runtimetest :: --compliance-level SHOULD

although I don't have any interesting SHOULD violations between my
test config.json and my host system.

[1]: https://testanything.org/tap-version-13-specification.html#todo
[2]: https://testanything.org/tap-version-13-specification.html#diagnostics

Signed-off-by: W. Trevor King <wking@tremily.us>
  • Loading branch information
wking committed Nov 24, 2017
1 parent 8e3ecf4 commit 9422eec
Showing 1 changed file with 28 additions and 31 deletions.
59 changes: 28 additions & 31 deletions cmd/runtimetest/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -891,46 +891,43 @@ func run(context *cli.Context) error {
complianceLevel = rfc2119.Must
logrus.Warningf("%s, using 'MUST' by default.", err.Error())
}
var validationErrors error
for _, v := range defaultValidations {
err := v.test(spec)
t.Ok(err == nil, v.description)
if err != nil {
if e, ok := err.(*specerror.Error); ok && e.Err.Level < complianceLevel {
continue
}
validationErrors = multierror.Append(validationErrors, err)
}
}

if platform == "linux" || platform == "solaris" {
for _, v := range posixValidations {
err := v.test(spec)
t.Ok(err == nil, v.description)
if err != nil {
if e, ok := err.(*specerror.Error); ok && e.Err.Level < complianceLevel {
continue
}
validationErrors = multierror.Append(validationErrors, err)
}
}
validations := defaultValidations
if platform == "linux" {
validations = append(validations, posixValidations...)
validations = append(validations, linuxValidations...)
} else if platform == "solaris" {
validations = append(validations, posixValidations...)
}

if platform == "linux" {
for _, v := range linuxValidations {
err := v.test(spec)
t.Ok(err == nil, v.description)
if err != nil {
if e, ok := err.(*specerror.Error); ok && e.Err.Level < complianceLevel {
continue
for _, v := range validations {
err := v.test(spec)
if err == nil {
t.Pass(v.description)
} else {
merr, ok := err.(*multierror.Error)
if ok {
for _, err = range merr.Errors {
if e, ok := err.(*rfc2119.Error); ok {
t.Ok(e.Level < complianceLevel, v.description)
} else {
t.Fail(v.description)
}
t.Diagnostic(err.Error())
}
} else {
if e, ok := err.(*rfc2119.Error); ok {
t.Ok(e.Level < complianceLevel, v.description)
} else {
t.Fail(v.description)
}
validationErrors = multierror.Append(validationErrors, err)
t.Diagnostic(err.Error())
}
}
}
t.AutoPlan()

return validationErrors
return nil
}

func main() {
Expand Down

0 comments on commit 9422eec

Please sign in to comment.