From dbaf6794c4f0321eba565a147721b66f74438ec5 Mon Sep 17 00:00:00 2001 From: Tim Kientzle Date: Sat, 18 Apr 2026 19:51:27 -0700 Subject: [PATCH] Document how to write new tests and how to build and run test suites I consider this an experiment to see if we can improve the quality of incoming PRs. --- SECURITY.md | 19 ++- doc/RUNNING_TESTS_LINUX.md | 70 +++++++++++ doc/WRITING_TESTS.md | 231 +++++++++++++++++++++++++++++++++++++ 3 files changed, 314 insertions(+), 6 deletions(-) create mode 100644 doc/RUNNING_TESTS_LINUX.md create mode 100644 doc/WRITING_TESTS.md diff --git a/SECURITY.md b/SECURITY.md index f2f60e79..33c37e47 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,9 +1,10 @@ # Security Policy -If you have discovered a security vulnerability in this project, please report it -privately. **Do not disclose it as a public issue.** This gives us time to work with you -to fix the issue before public exposure, reducing the chance that the exploit will be -used before a patch is released. +If you have discovered a security vulnerability in this project, +please report it privately. +**Do not disclose it as a public issue.** +This gives us time to work with you to fix the issue before public exposure, +reducing the chance that the exploit will be used before a patch is released. You may submit the report in the following ways: @@ -15,5 +16,11 @@ Please provide the following information in your report: - A description of the vulnerability and its impact - How to reproduce the issue -This project is maintained by volunteers on a reasonable-effort basis. As such, we ask -that you give us 90 days to work on a fix before public exposure. +While we welcome all reports, we can act most quickly if the report +includes a complete libarchive test case and a proposed fix. +See [doc/WRITING_TESTS.md](doc/WRITING_TESTS.md) for guidance on creating a reproduction test case. +You can provide the test case and proposed fix as separate patch files +prepared with `git format-patch` or `git diff`. + +This project is maintained by volunteers on a reasonable-effort basis. +We ask that you give us 90 days to work on a fix before public exposure. diff --git a/doc/RUNNING_TESTS_LINUX.md b/doc/RUNNING_TESTS_LINUX.md new file mode 100644 index 00000000..62b6d185 --- /dev/null +++ b/doc/RUNNING_TESTS_LINUX.md @@ -0,0 +1,70 @@ +--- +name: libarchive-running-tests-linux +description: Explains how to build and run the libarchive test suite on Linux. +--- + +WIP: This is still being developed. +Of course, we'll need other `RUNNING_TESTS_.md`, +as well as `BUILDING_.md` files with more details about various build options and setup. + +# Building and Running Tests on Linux + +The following is a summary of the information in [doc/BUILDING_LINUX.md](BUILDING_LINUX.md). + +### Configuring and Building the Tests + +We recommend using the CMake build system with the Ninja build tool. +A parallel build system based on GNU autoconf is also available. + +To configure with CMake: +```bash +cmake -G Ninja -B _build -S . +``` +(Omit `-G Ninja` to use the platform default build system +-- usually `make` -- instead of Ninja.) + +To build all of libarchive, including the test suites, +using whatever build system you selected during configuration: +```bash +cmake --build _build +``` + +### Testing a component + +There is a single test executable for each major libarchive component. + +To test the libarchive library: +```bash +./_build/bin/libarchive_test +``` + +To test other components: +```bash +./_build/bin/bsdcat_test +./_build/bin/bsdcpio_test +./_build/bin/bsdtar_test +./_build/bin/bsdunzip_test +``` + +Each test executables runs a series of tests on the corresponding component. +Each test runs in its own temporary directory; +if the test fails, the temporary directory is left behind so you can inspect the contents. + +Reference files are loaded from a specific hardcoded path. +This assumes the current working directory is the root of the libarchive source tree. +If you need to run a test executable from a different directory, +use the `-r` option to provide a path to the reference files. + +Use the `-h` (help) option for more details or `-v` (verbose) for more detailed output. + +### Running a single test + +You can run a single test by name +```bash +./_build/bin/libarchive_test test_read_format_zip +``` +or by number +```bash +./_build/bin/libarchive_test 307 +./_build/bin/libarchive_test 446-448 +``` diff --git a/doc/WRITING_TESTS.md b/doc/WRITING_TESTS.md new file mode 100644 index 00000000..4a76cd1b --- /dev/null +++ b/doc/WRITING_TESTS.md @@ -0,0 +1,231 @@ +--- +name: libarchive-writing-tests +description: Guide for writing and maintaining libarchive unit tests. +--- + +# Writing Libarchive Unit Tests + +Each libarchive component includes a suite of unit tests. +These are located in the `test` directory underneath the component directory +(`libarchive/test`, `tar/test`, etc). + +The most common reason that we fail to accept patches to libarchive +is because the patch lacks suitable new tests. +Almost all changes should be accompanied with a new test case, +and if the change is non-trivial it will likely require several +tests to verify the full range of new behavior. +A new format or filter will likely require dozens of new tests. + +The rest of this file explains how to write a new test case and +add it to the existing test suite. +The RUNNING_TESTS documentation corresponding to your platform +describes how to build and run the test suite. + +## Creating a New Test + +A "test" in libarchive is a C function defined with the `DEFINE_TEST(test_name)` macro. +While most test files contain only a single test, it is possible to include multiple tests in a single source file. +Many tests also involve an associated reference or sample input file, which is typically stored in the repository in UUencoded format. + +### 1. Basic Template +```c +#include "test.h" + +/* The test name typically matches the name of the `.c` source file. */ +DEFINE_TEST(test_foo) +{ + /* The file in the repository is named "test_foo.tar.uu". */ + /* The `refname` here omits the `.uu` suffix. */ + /* The base filename should match the test name. */ + const char *refname = "test_foo.tar"; + + struct archive *a; + struct archive_entry *ae; + int r; + + /* Each test runs in a private temporary directory. */ + + /* If you have a binary archive sample, extract it first. */ + extract_reference_file(refname); + + /* Create and configure a suitable archive handle. */ + assert((a = archive_read_new()) != NULL); + assertEqualIntA(a, ARCHIVE_OK, archive_read_support_filter_all(a)); + assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_all(a)); + + /* Open the extracted reference file. */ + assertEqualIntA(a, ARCHIVE_OK, archive_read_open_filename(a, refname, 10240)); + + /* Perform whatever tests are appropriate. */ + + /* For example, when fixing a crash in libarchive, it + * may suffice for a test to simply read all the entries and + * their contents, as shown below. + * If errors are expected, you may need to unroll the following + * in order to perform specific checks for each entry. */ + while ((r = archive_read_next_header(a, &ae)) == ARCHIVE_OK) { + const void *pv; + size_t s; + int64_t o; + + while ((r == archive_read_data_block(a, &pv, &s, &o)) == ARCHIVE_OK) { + /* Verify the data contents if appropriate. */ + } + /* Verify that we exited the loop above on EOF, not on error. */ + assertEqualIntA(a, ARCHIVE_EOF, r); + } + + /* We exited the loop above, verify that it's EOF and not an error. */ + assertEqualIntA(a, ARCHIVE_EOF, r); + + /* Best practice: Close and free separately to get better + * error reporting if close fails. */ + assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a)); + assertEqualInt(ARCHIVE_OK, archive_read_free(a)); +} +``` + +### 2. Reference Files (Compatibility Tests) +For testing compatibility with existing archives: +1. Place the sample data in the `test` directory alongside the `.c` file. +2. Encode it using `uuencode`: `uuencode test_foo.tar test_foo.tar > test_foo.tar.uu`. +3. Use `extract_reference_file("test_foo.tar")` in your test to decode it into the temporary work directory. + +### 3. File and Function Naming + +The basic naming convention is `test___.c` where: +- **Functionality:** Indicates general functionality (e.g., `read`, `write`, `archive`, `entry`, `compat`). +- **Format/API:** The specific format or API being exercised (e.g., `format_tar`, `filter_gzip`, `next_header`, `acl`). +- **Detail:** Additional context to clarify the purpose (e.g., `empty`, `large`, `mac_metadata`). + +**Example names from the current suite:** +- `test_read_format_zip_mac_metadata.c` +- `test_write_disk_perms.c` +- `test_archive_read_next_header_empty.c` + +### 4. Registering the Test + +Adding the new files to build system is sufficient to have them built +and run with the rest of the suite: + +1. `Makefile.am` in the root: + - Add `.c` files to `libarchive_test_SOURCES`. + - Add `.uu` reference files to `libarchive_test_EXTRA_DIST`. + - **Note:** Please maintain alphabetical order in these lists. +2. `CMakeLists.txt` in the relevant directory (e.g., `libarchive/test/CMakeLists.txt`): + - Add `.c` files to `libarchive_test_SOURCES`. + - **Note:** `.uu` files do NOT need to be listed in `CMakeLists.txt`. + +--- + +## Assertion Macros + +Always use the specialized assertion macros to ensure failures are actionable. +The full list of available macros is defined in `test_utils/test_common.h`. + +| Macro | Description | +|-------|-------------| +| `assert(expr)` | Standard boolean assertion. | +| `assertEqualInt(v1, v2)` | Asserts two integers are equal. Prints both values on failure. | +| `assertEqualString(s1, s2)` | Asserts two strings are equal. Prints both on failure. | +| `assertEqualMem(p1, p2, n)` | Asserts two memory blocks are equal. Provides a hexdump on failure. | +| `assertA(expr)` | Like `assert`, but also prints `archive_error_string(a)` on failure. | +| `assertEqualIntA(a, v1, v2)` | Like `assertEqualInt`, but includes archive error context. | +| `failure(fmt, ...)` | **Critical:** Use before an assertion to provide context. The message is only shown if the following assertion fails. | + +**Example usage:** +```c + /* Read first entry and verify metadata. */ + assertEqualIntA(a, ARCHIVE_OK, r = archive_read_next_header(a, &ae)); + if (r != ARCHIVE_OK) { ... end the test ... } + + assertEqualString("test_file.txt", archive_entry_pathname(ae)); + assertEqualInt(1197179003, archive_entry_mtime(ae)); + assertEqualInt(1000, archive_entry_uid(ae)); + assertEqualString("tim", archive_entry_uname(ae)); + assertEqualInt(0100644, archive_entry_mode(ae)); +``` + +--- + +## Working with Files and Data + +Many libarchive tests perform all operations entirely in memory. +However, when testing archive extraction, you must verify the resulting files on disk. +Similarly, when testing archive creation, you may need to create files on disk to be archived. +The harness provides a variety of file utilities expressed as assertions. + +This includes assertions that create or modify files: +- `assertMakeFile(path, mode, contents)` +- `assertMakeDir(path, mode)` +- `assertChmod(pathname, mode)` + +And assertions that verify particular file properties, such as: +- `assertFileExists(path)` +- `assertFileSize(path, expected_size)` +- `assertTextFileContents(expected_text, path)` + +The full set of asertions is listed in `test_utils/test_common.h`. +You can search the existing tests to find examples of their usage. + +--- + +## Platform and Environment Utilities + +Some checks can only be done if the underlying system has certain +capabilities. + +Utilities to check capabilities of the current system include: +- `canSymlink()` +- `canGzip()` +- `canRunCommand("cmd", NULL)`. + +These can be used to circumvent specific individual checks: + +```c +if (canSymlink()) { + skipping("Symlinks not supported on this platform"); + /* ... verify that symlink entries were correctly restored */ +} +``` + +Or to abort an entire test: +```c +if (!canSymlink()) { + skipping("Symlinks not supported on this platform"); + return; +} +``` + +--- + +## General Best Practices + +Some pointers for making more effective tests: + +- **Use assertions liberally:** + Most lines of code in a test use `assert` macros to + verify the expected result. +- **Always Clean Up:** + Call `archive_read_free()` or `archive_write_free()` on every exit + path, including failures and premature exits. +- **Cross-Platform Portability:** + Do not assume POSIX features like `symlink` or `chown`. + Use the `can...()` helpers to verify system support before + performing platform-specific checks. +- **Verify the Fix:** + Run your new test against the code before and after any + other changes to ensure the test reproduces the issue and that + your fix addresses it. +- **Verify Error Details:** + When an API returns `ARCHIVE_WARN` or `ARCHIVE_FATAL`, + verify the specific error code from `archive_errno()`. + However, you should not always check the result of `archive_error_string()`, + as the specific text can evolve over time. + Generally, there should be one test each time a new error path is added + that verifies the error string. +- **Match the Existing Style:** + Changes to existing files should always match the indentation + and style of that file. + New files should follow the "BSD" formatting convention, + with 8-character hard tabs for indentation.