From cf360fad9224ac0a8d2d4709a8c973be0ab169f2 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Tue, 23 Nov 2021 09:45:01 +0000 Subject: [PATCH] docs: Move unit test advice doc from tests repo Unit tests necessarily need to be maintained with the code they test so it makes sense to keep the Unit Test Advice document into the main repo since that is where the majority of unit tests reside. Note: The [`Unit-Test-Advice.md` file](https://github.com/kata-containers/tests/blob/main/Unit-Test-Advice.md) was copied from the `tests` repo when it's `HEAD` was https://github.com/kata-containers/tests/commit/38855f1f40120f4f009fa30061e9affc07930640. Signed-off-by: James O. D. Hunt --- docs/README.md | 4 + docs/Unit-Test-Advice.md | 333 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 337 insertions(+) create mode 100644 docs/Unit-Test-Advice.md diff --git a/docs/README.md b/docs/README.md index fcb76cf344..d1efc14ba5 100644 --- a/docs/README.md +++ b/docs/README.md @@ -52,6 +52,10 @@ Documents that help to understand and contribute to Kata Containers. * [How to contribute to Kata Containers](https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md) * [Code of Conduct](../CODE_OF_CONDUCT.md) +## Help Writing Unit Tests + +* [Unit Test Advice](Unit-Test-Advice.md) + ## Help Improving the Documents * [Documentation Requirements](Documentation-Requirements.md) diff --git a/docs/Unit-Test-Advice.md b/docs/Unit-Test-Advice.md new file mode 100644 index 0000000000..88663c9760 --- /dev/null +++ b/docs/Unit-Test-Advice.md @@ -0,0 +1,333 @@ +# Unit Test Advice + +* [Overview](#overview) +* [Assertions](#assertions) + * [golang assertions](#golang-assertions) + * [rust assertions](#rust-assertions) +* [Table driven tests](#table-driven-tests) + * [golang table driven tests](#golang-table-driven-tests) + * [rust table driven tests](#rust-table-driven-tests) +* [Temporary files](#temporary-files) + * [golang temporary files](#golang-temporary-files) + * [rust temporary files](#rust-temporary-files) +* [User running the test](#user-running-the-test) + * [running golang tests as different users](#running-golang-tests-as-different-users) + * [running rust tests as different users](#running-rust-tests-as-different-users) + +## Overview + +This document offers advice on writing a Unit Test (UT) in +[`golang`](https://golang.org) and [`rust`](https://www.rust-lang.org). + +## Assertions + +### golang assertions + +Use the `testify` assertions package to create a new assertion object as this +keeps the test code free from distracting `if` tests: + +```go +func TestSomething(t *testing.T) { + assert := assert.New(t) + + err := doSomething() + assert.NoError(err) +} +``` + +### rust assertions + +Use the standard set of `assert!()` macros. + +## Table driven tests + +Try to write tests using a table-based approach. This allows you to distill +the logic into a compact table (rather than spreading the tests across +multiple test functions). It also makes it easy to cover all the +interesting boundary conditions: + +### golang table driven tests + +Assume the following function: + +```go +// The function under test. +// +// Accepts a string and an integer and returns the +// result of sticking them together separated by a dash as a string. +func joinParamsWithDash(str string, num int) (string, error) { + if str == "" { + return "", errors.New("string cannot be blank") + } + + if num <= 0 { + return "", errors.New("number must be positive") + } + + return fmt.Sprintf("%s-%d", str, num), nil +} +``` + +A table driven approach to testing it: + +```go +import ( + "testing" + "github.com/stretchr/testify/assert" +) + +func TestJoinParamsWithDash(t *testing.T) { + assert := assert.New(t) + + // Type used to hold function parameters and expected results. + type testData struct { + param1 string + param2 int + expectedResult string + expectError bool + } + + // List of tests to run including the expected results + data := []testData{ + // Failure scenarios + {"", -1, "", true}, + {"", 0, "", true}, + {"", 1, "", true}, + {"foo", 0, "", true}, + {"foo", -1, "", true}, + + // Success scenarios + {"foo", 1, "foo-1", false}, + {"bar", 42, "bar-42", false}, + } + + // Run the tests + for i, d := range data { + // Create a test-specific string that is added to each assert + // call. It will be displayed if any assert test fails. + msg := fmt.Sprintf("test[%d]: %+v", i, d) + + // Call the function under test + result, err := joinParamsWithDash(d.param1, d.param2) + + // update the message for more information on failure + msg = fmt.Sprintf("%s, result: %q, err: %v", msg, result, err) + + if d.expectError { + assert.Error(err, msg) + + // If an error is expected, there is no point + // performing additional checks. + continue + } + + assert.NoError(err, msg) + assert.Equal(d.expectedResult, result, msg) + } +} +``` + +### rust table driven tests + +Assume the following function: + +```rust +// Convenience type to allow Result return types to only specify the type +// for the true case; failures are specified as static strings. +pub type Result = std::result::Result; + +// The function under test. +// +// Accepts a string and an integer and returns the +// result of sticking them together separated by a dash as a string. +fn join_params_with_dash(str: &str, num: i32) -> Result { + if str == "" { + return Err("string cannot be blank"); + } + + if num <= 0 { + return Err("number must be positive"); + } + + let result = format!("{}-{}", str, num); + + Ok(result) +} + +``` + +A table driven approach to testing it: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_join_params_with_dash() { + // This is a type used to record all details of the inputs + // and outputs of the function under test. + #[derive(Debug)] + struct TestData<'a> { + str: &'a str, + num: i32, + result: Result, + } + + // The tests can now be specified as a set of inputs and outputs + let tests = &[ + // Failure scenarios + TestData { + str: "", + num: 0, + result: Err("string cannot be blank"), + }, + TestData { + str: "foo", + num: -1, + result: Err("number must be positive"), + }, + + // Success scenarios + TestData { + str: "foo", + num: 42, + result: Ok("foo-42".to_string()), + }, + TestData { + str: "-", + num: 1, + result: Ok("--1".to_string()), + }, + ]; + + // Run the tests + for (i, d) in tests.iter().enumerate() { + // Create a string containing details of the test + let msg = format!("test[{}]: {:?}", i, d); + + // Call the function under test + let result = join_params_with_dash(d.str, d.num); + + // Update the test details string with the results of the call + let msg = format!("{}, result: {:?}", msg, result); + + // Perform the checks + if d.result.is_ok() { + assert!(result == d.result, msg); + continue; + } + + let expected_error = format!("{}", d.result.as_ref().unwrap_err()); + let actual_error = format!("{}", result.unwrap_err()); + assert!(actual_error == expected_error, msg); + } + } +} +``` + +## Temporary files + +Always delete temporary files on success. + +### golang temporary files + +```go +func TestSomething(t *testing.T) { + assert := assert.New(t) + + // Create a temporary directory + tmpdir, err := ioutil.TempDir("", "") + assert.NoError(err) + + // Delete it at the end of the test + defer os.RemoveAll(tmpdir) + + // Add test logic that will use the tmpdir here... +} +``` + +### rust temporary files + +Use the `tempfile` crate which allows files and directories to be deleted +automatically: + +```rust +#[cfg(test)] +mod tests { + use tempfile::tempdir; + + #[test] + fn test_something() { + + // Create a temporary directory (which will be deleted automatically + let dir = tempdir().expect("failed to create tmpdir"); + + let filename = dir.path().join("file.txt"); + + // create filename ... + } +} + +``` + +## User running the test + +[Unit tests are run *twice*](https://github.com/kata-containers/tests/blob/main/.ci/go-test.sh): + +- as the current user +- as the `root` user (if different to the current user) + +When writing a test consider which user should run it; even if the code the +test is exercising runs as `root`, it may be necessary to *only* run the test +as a non-`root` for the test to be meaningful. + +Some repositories already provide utility functions to skip a test: + +- if running as `root` +- if not running as `root` + +### running golang tests as different users + +The runtime repository has the most comprehensive set of skip abilities. See: + +- https://github.com/kata-containers/kata-containers/tree/main/src/runtime/pkg/katatestutils + +### running rust tests as different users + +One method is to use the `nix` crate along with some custom macros: + +``` +#[cfg(test)] +mod tests { + #[allow(unused_macros)] + macro_rules! skip_if_root { + () => { + if nix::unistd::Uid::effective().is_root() { + println!("INFO: skipping {} which needs non-root", module_path!()); + return; + } + }; + } + + #[allow(unused_macros)] + macro_rules! skip_if_not_root { + () => { + if !nix::unistd::Uid::effective().is_root() { + println!("INFO: skipping {} which needs root", module_path!()); + return; + } + }; + } + + #[test] + fn test_that_must_be_run_as_root() { + // Not running as the superuser, so skip. + skip_if_not_root!(); + + // Run test *iff* the user running the test is root + + // ... + } +} +```