docs: Add unit testing presentation

Add the Kata Containers unit testing presentation I gave to the Kata
outreach students as this may be of some use to others.

Fixes: #3781

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

docs/README.md

@@ -58,6 +58,7 @@ Documents that help to understand and contribute to Kata Containers.
 ## Help Writing Unit Tests
 
 * [Unit Test Advice](Unit-Test-Advice.md)
+* [Unit testing presentation](presentations/unit-testing/kata-containers-unit-testing.md)
 
 ## Help Improving the Documents
 
@@ -72,6 +73,10 @@ Documents that help to understand and contribute to Kata Containers.
 * [Release strategy](Stable-Branch-Strategy.md)
 * [Release Process](Release-Process.md)
 
+## Presentations
+
+* [Presentations](presentations)
+
 ## Website Changes
 
 If you have a suggestion for how we can improve the

docs/presentations/README.md

@@ -0,0 +1,3 @@
+# Kata Containers presentations
+
+* [Unit testing](unit-testing)

docs/presentations/unit-testing/README.md

@@ -0,0 +1,14 @@
+# Kata Containers unit testing presentation
+
+## Markdown version
+
+See [the Kata Containers unit testing presentation](kata-containers-unit-testing.md).
+
+### To view as an HTML presentation
+
+```bash
+$ infile="kata-containers-unit-testing.md"
+$ outfile="/tmp/kata-containers-unit-testing.html"
+$ pandoc -s --metadata title="Kata Containers unit testing" -f markdown -t revealjs --highlight-style="zenburn" -i -o "$outfile" "$infile"
+$ xdg-open "file://$outfile"
+```

docs/presentations/unit-testing/kata-containers-unit-testing.md

@@ -0,0 +1,335 @@
+## Why write unit tests?
+
+- Catch regressions
+
+- Improve the code being tested
+
+  Structure, quality, security, performance, "shakes out" implicit
+  assumptions, _etc_
+
+- Extremely instructive
+
+  Once you've fully tested a single function, you'll understand that
+  code very well indeed.
+
+## Why write unit tests? (continued)
+
+- Fun!
+
+  Yes, really! Don't believe me? Try it! ;)
+
+## Run all Kata Containers agent unit tests
+
+As an example, to run all agent unit tests:
+
+```bash
+$ cd $GOPATH/src/github.com/kata-containers/kata-containers
+$ cd src/agent
+$ make test
+```
+
+## List all unit tests
+
+- Identify the full name of all the tests _in the current package_:
+
+  ```bash
+  $ cargo test -- --list
+  ```
+
+- Identify the full name of all tests in the `foo` "local crate"
+  (sub-directory containing another `Cargo.toml` file):
+
+  ```bash
+  $ cargo test -p "foo" -- --list
+  ```
+
+## Run a single unit test
+
+- Run a test in the current package in verbose mode:
+
+  ```bash
+  # Example 
+  $ test="config::tests::test_get_log_level"
+
+  $ cargo test "$test" -vv -- --exact --nocapture
+  ```
+
+## Test coverage setup
+
+```bash
+$ cargo install cargo-tarpaulin
+```
+
+## Show test coverage
+
+```bash
+$ cd $GOPATH/src/github.com/kata-containers/kata-containers/src/agent
+$ cargo -v tarpaulin --all-features --run-types AllTargets --count --force-clean -o Html
+$ xdg-open "file://$PWD/tarpaulin-report.html"
+```
+
+## Testability (part 1)
+
+- To be testable, a function should:
+  - Not be "too long" (say >100 lines).
+  - Not be "too complex" (say >3 levels of indentation).
+  - Should return a `Result` or an `Option` so error paths
+    can be tested.
+
+- If functions don't conform, they need to be reworked (refactored)
+  before writing tests.
+
+## Testability (part 2)
+
+- Some functions can't be fully tested.
+- However, you _can_ test the initial code that checks
+  the parameter values (test error paths only).
+
+## Writing new tests: General advice (part 1)
+
+- KISS: Keep It Simple Stupid
+
+  You don't get extra points for cryptic code.
+
+- DRY: Don't Repeat Yourself
+
+  Make use of existing facilities (don't "re-invert the wheel").
+
+- Read the [unit test advice document](https://github.com/kata-containers/kata-containers/blob/main/docs/Unit-Test-Advice.md)
+
+## Writing new tests: General advice (part 2)
+
+- Attack the function in all possible ways
+
+- Use the _table driven_ approach:
+  - Simple
+  - Compact
+  - Easy to debug
+  - Makes boundary analysis easy
+  - Encourages functions to be testable
+
+## Writing new tests: Specific advice (part 1)
+
+- Create a new "`tests`" module if necessary.
+- Give each test function a "`test_`" prefix.
+- Add the "`#[test]`" annotation on each test function.
+
+## Writing new tests: Specific advice (part 2)
+
+- If you need to `use` (import) packages for the tests,
+  _only do it in the `tests` module_:
+  ```rust
+  use some_test_pkg::{foo, bar}; // <-- Not here
+
+  #[cfg(test)]
+  mod tests {
+    use super::*;
+    use some_test_pkg:{foo, bar}; // <-- Put it here
+  }
+  ```
+
+## Writing new tests: Specific advice (part 3)
+
+- You can add test-specific dependencies in `Cargo.toml`:
+  ```toml
+  [dev-dependencies]
+  serial_test = "0.5.1"
+  ```
+
+## Writing new tests: Specific advice (part 4)
+
+- Don't add in lots of error handling code: let the test panic!
+  ```rust
+  // This will panic if the unwrap fails.
+  // - NOT acceptable generally for production code.
+  // - PERFECTLY acceptable for test code since:
+  //   - Keeps the test code simple.
+  //   - Rust will detect the panic and fail the test.
+  let result = func().unwrap();
+  ```
+
+## Debugging tests (part 1)
+
+- Comment out all tests in your `TestData` array apart from the failing test.
+
+- Add temporary `println!("FIXME: ...")` statements in the code.
+
+- Set `RUST_BACKTRACE=full` before running `cargo test`.
+
+## Debugging tests (part 2)
+
+- Use a debugger (not normally necessary though):
+  ```bash
+  # Disable optimisation
+  $ RUSTFLAGS="-C opt-level=0" cargo test --no-run
+
+  # Find the test binary
+  $ test_binary=$(find target/debug/deps | grep "kata_agent-[a-z0-9][a-z0-9]*$" | tail -1)
+
+  $ rust-gdb "$test_binary"
+  ```
+
+## Useful tips
+
+- Always start a test with a "clean environment":
+
+  Create new set of objects / files / directories / _etc_
+  for each test.
+
+- Mounts
+  - Linux allows mounts on top of existing mounts.
+  - Bind mounts and read-only mounts can be useful.
+
+## Gotchas (part 1)
+
+If a test runs successfully _most of the time_:
+
+- Review the test logic.
+
+- Add a `#[serial]` annotation on the test function
+  Requires the `serial_test` package in the `[dev-dependencies]`
+  section of `Cargo.toml`.
+
+  If this makes it work the test is probably sharing resources with
+  another task (thread).
+
+## Gotchas (part 2)
+
+If a test works locally but fails in the CI, consider the following
+attributes of each environment (local and CI):
+
+- The version of rust being used.
+- The hardware architecture.
+- Number (and spec) of the CPUs.
+
+## Gotchas (part 3)
+
+If in doubt, look at the
+["test artifacts" attached to the failing CI test](http://jenkins.katacontainers.io).
+
+## Before raising a PR
+
+- Remember to check that the test runs locally:
+  - As a non-privileged user.
+  - As the `root` user (carefully!)
+
+- Run the [static checker](https://github.com/kata-containers/tests/blob/main/.ci/static-checks.sh)
+  on your changes.
+
+  Checks formatting and many other things.
+
+## If in doubt
+
+- Ask for help! ;)
+
+## Quiz 1
+
+What's wrong with this function?
+
+```rust
+fn foo(config: &Config, path_prefix: String, container_id: String, pid: String) -> Result<()> {
+    let mut full_path = format!("{}/{}", path_prefix, container_id);
+
+    let _ = remove_recursively(&mut full_path);
+
+    write_number_to_file(pid, full_path);
+
+    Ok(())
+}
+```
+
+## Quiz 1: Answers (part 1)
+
+- No check that `path_prefix`, `container_id` and `pid` are not `""`.
+- No check that `path_prefix` is absolute.
+- No check that `container_id` does not contain slashes / contains only valid characters.
+- Result of `remove_recursively()` discarded.
+- `remove_recursively()` _may_ modify `full_path` without `foo()` knowing!
+
+## Quiz 1: Answers (part 2)
+
+- Why is `pid` not a numeric?
+- No check to ensure the PID is positive.
+- No check to recreate any directories in the original `path_prefix`.
+- `write_number_to_file()` could fail so why doesn't it return a value?
+- The `config` parameter is unused.
+
+## Quiz 1: What if...
+
+Imagine if the caller managed to do this:
+
+```rust
+foo(config, "", "sbin/init", r#"#!/bin/sh\n/sbin/reboot"#);
+```
+
+## Quiz 2
+
+What makes this function difficult to test?
+
+```rust
+fn get_user_id(username: String) -> i32 {
+    let line = grep_file(username, "/etc/passwd").unwrap();
+    let fields = line.split(':');
+
+    let uid = fields.nth(2).ok_or("failed").unwrap();
+
+    uid.parse::<i32>()
+}
+```
+
+## Quiz 2: Answers (part 1)
+
+- Unhelpful error message ("failed").
+
+- Panics on error! Return a `Result` instead!
+
+- UID's cannot be negative so function should return an unsigned
+  value.
+
+## Quiz 2: Answers (part 2)
+
+- Hard-coded filename.
+
+  This would be better:
+
+  ```rust
+  const PASSWD_DB: &str = "/etc/passwd";
+
+  // Test code can now pass valid and invalid files!
+  fn get_user_id(filename: String, username: String) -> i32 {
+    // ...
+  }
+
+  let id = get_user_id(PASSWD_DB, username);
+  ```
+
+## Quiz 3
+
+What's wrong with this test code?
+
+```rust
+let mut obj = Object::new();
+
+// Sanity check
+assert_eq!(obj.num, 0);
+assert_eq!(obj.wibble, false);
+
+// Test 1
+obj->foo_method(7);
+assert_eq!(obj.num, 7);
+
+// Test 2
+obj->bar_method(true);
+assert_eq!(obj.wibble, true);
+```
+
+## Quiz 3: Answers
+
+- The test code is "fragile":
+  - The 2nd test re-uses the object created in the first test.
+
+## Finally
+
+- [We need a GH action to run the unit tests](https://github.com/kata-containers/kata-containers/issues/2934)
+
+  Needs to fail PRs that decrease test coverage<br/> by "x%".