From fab1de635eba1d316c5637e88566a56755978861 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Fri, 8 Jun 2018 16:04:46 +0200 Subject: [PATCH] validation: use t.Fail when checking for main test errors So far `util.Fatal()`, which calls `os.Exit(1)`, has been widely used to exit immediately when a critical error happened. Its downside is though that no further function can be called after that moment, even defer functions cannot be called. That's fine if there's nothing to clean up or the test is simple enough. Though it could be an issue, for example, when a test should print out TAP output by calling `t.AutoPlan()`, or when it should clean up something. So let's try to use `util.Fatal()` only for errors from critical cases like errors from `util.GetDefaultGenerator()` or `util.PrepareBundle()`. In case of main test errors, use instead `t.Fail(err.Error())` to print out errors in TAP outputs. Partly addresses https://github.com/opencontainers/runtime-tools/issues/582 /cc @liangchenye Signed-off-by: Dongsu Park --- .../config_updates_without_affect.go | 2 +- validation/delete/delete.go | 2 +- validation/hooks_stdin/hooks_stdin.go | 2 +- validation/hostname/hostname.go | 2 +- validation/kill_no_effect/kill_no_effect.go | 2 +- validation/killsig/killsig.go | 1 + validation/linux_ns_itype/linux_ns_itype.go | 2 +- validation/linux_ns_nopath/linux_ns_nopath.go | 2 +- validation/start/start.go | 20 +++++++++++-------- 9 files changed, 20 insertions(+), 15 deletions(-) diff --git a/validation/config_updates_without_affect/config_updates_without_affect.go b/validation/config_updates_without_affect/config_updates_without_affect.go index 0625c8b35..77c02ed19 100644 --- a/validation/config_updates_without_affect/config_updates_without_affect.go +++ b/validation/config_updates_without_affect/config_updates_without_affect.go @@ -82,7 +82,7 @@ func main() { err = r.Delete() } if err != nil { - util.Fatal(err) + t.Fail(err.Error()) } t.AutoPlan() diff --git a/validation/delete/delete.go b/validation/delete/delete.go index 1eef5bb53..98374e3d2 100644 --- a/validation/delete/delete.go +++ b/validation/delete/delete.go @@ -85,7 +85,7 @@ func main() { util.WaitingForStatus(testRuntime, util.LifecycleStatusStopped, time.Second*10, time.Second*1) err = testRuntime.Delete() if err != nil { - util.Fatal(err) + t.Fail(err.Error()) } } } diff --git a/validation/hooks_stdin/hooks_stdin.go b/validation/hooks_stdin/hooks_stdin.go index 7e68ffab4..aef63161a 100644 --- a/validation/hooks_stdin/hooks_stdin.go +++ b/validation/hooks_stdin/hooks_stdin.go @@ -140,7 +140,7 @@ func main() { err = util.RuntimeLifecycleValidate(config) if err != nil { - util.Fatal(err) + t.Fail(err.Error()) } expectedState := rspecs.State{ diff --git a/validation/hostname/hostname.go b/validation/hostname/hostname.go index 1cce37d8e..ec1b6e785 100644 --- a/validation/hostname/hostname.go +++ b/validation/hostname/hostname.go @@ -40,7 +40,7 @@ func main() { for _, h := range hostnames { if err := testHostname(t, h); err != nil { - util.Fatal(err) + t.Fail(err.Error()) } } } diff --git a/validation/kill_no_effect/kill_no_effect.go b/validation/kill_no_effect/kill_no_effect.go index 25549ea50..601cdf537 100644 --- a/validation/kill_no_effect/kill_no_effect.go +++ b/validation/kill_no_effect/kill_no_effect.go @@ -57,7 +57,7 @@ func main() { } err = util.RuntimeLifecycleValidate(config) if err != nil && err != targetErr { - util.Fatal(err) + t.Fail(err.Error()) } else { util.SpecErrorOK(t, err == nil, targetErr, nil) } diff --git a/validation/killsig/killsig.go b/validation/killsig/killsig.go index bdef16f92..d0cde75d1 100644 --- a/validation/killsig/killsig.go +++ b/validation/killsig/killsig.go @@ -31,6 +31,7 @@ func main() { containerID := uuid.NewV4().String() sigConfig, err := util.GetDefaultGenerator() if err != nil { + os.RemoveAll(bundleDir) util.Fatal(err) } rootDir := filepath.Join(bundleDir, sigConfig.Spec().Root.Path) diff --git a/validation/linux_ns_itype/linux_ns_itype.go b/validation/linux_ns_itype/linux_ns_itype.go index f46bf0da8..13b9adc6e 100644 --- a/validation/linux_ns_itype/linux_ns_itype.go +++ b/validation/linux_ns_itype/linux_ns_itype.go @@ -124,7 +124,7 @@ func main() { err := testNamespaceInheritType(t) if err != nil { - util.Fatal(err) + t.Fail(err.Error()) } t.AutoPlan() diff --git a/validation/linux_ns_nopath/linux_ns_nopath.go b/validation/linux_ns_nopath/linux_ns_nopath.go index 2b6cab496..ae6e8ec90 100644 --- a/validation/linux_ns_nopath/linux_ns_nopath.go +++ b/validation/linux_ns_nopath/linux_ns_nopath.go @@ -125,7 +125,7 @@ func main() { err := testNamespaceNoPath(t) if err != nil { - util.Fatal(err) + t.Fail(err.Error()) } t.AutoPlan() diff --git a/validation/start/start.go b/validation/start/start.go index 8b4c01951..473f96a81 100644 --- a/validation/start/start.go +++ b/validation/start/start.go @@ -17,6 +17,7 @@ import ( func main() { t := tap.New() t.Header(0) + defer t.AutoPlan() bundleDir, err := util.PrepareBundle() if err != nil { @@ -54,7 +55,8 @@ func main() { err = r.Create() if err != nil { - util.Fatal(err) + t.Fail(err.Error()) + return } _, err = os.Stat(output) // check the existence of the output file @@ -67,7 +69,8 @@ func main() { } else { err = util.WaitingForStatus(r, util.LifecycleStatusStopped, time.Second*10, time.Second*1) if err != nil { - util.Fatal(err) + t.Fail(err.Error()) + return } outputData, outputErr := ioutil.ReadFile(output) // check the output @@ -81,7 +84,8 @@ func main() { err = util.WaitingForStatus(r, util.LifecycleStatusStopped, time.Second*10, time.Second*1) if err != nil { - util.Fatal(err) + t.Fail(err.Error()) + return } outputData, outputErr := ioutil.ReadFile(output) @@ -90,7 +94,8 @@ func main() { err = r.Delete() if err != nil { - util.Fatal(err) + t.Fail(err.Error()) + return } g.Spec().Process = nil @@ -100,7 +105,8 @@ func main() { } err = r.Create() if err != nil { - util.Fatal(err) + t.Fail(err.Error()) + return } err = r.Start() @@ -110,8 +116,6 @@ func main() { err = r.Delete() } if err != nil { - util.Fatal(err) + t.Fail(err.Error()) } - - t.AutoPlan() }