Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

netdog: Make net_config unit tests reusable across versions #2385

Merged
merged 2 commits into from
Sep 1, 2022

Conversation

zmrow
Copy link
Contributor

@zmrow zmrow commented Aug 29, 2022

Issue number:
Related to #2204

Description of changes:
It's probably easiest to read this PR by commit, since the second commit has a decent amount of noise from moving the test_data files around.

As we add versions to net_config we need to be able to run the same set of unit tests across multiple versions. Net config versions are typically "additive", meaning that new versions add support for a new feature while continuing to support previous features. Rust's default test tooling doesn't allow parameterizing tests, and while one could loop over versions within a single test, the output of the cargo test tooling is less than satisfactory since you get a single line for a test.

The objectives:

  • cargo test output must show a line for each network config version/test
  • Easily run a set of tests across multiple versions; no copy/paste of test code between versions
  • It must be easy to add new tests for upcoming versions.

This PR achieves the above objective by moving the actual test code into macros. Each macro contains a group of tests for a given feature. The macros are declarative and generate the appropriate test code, using the version as a key. Each macro's associated test data files are in a similarly named directory and templated, using the version.

Using the test macros in each net config version involves importing the macro inside the #[cfg(test)] block, and invoking it:

// This is an example for V1 net config
#[cfg(test)]
mod tests {
    use crate::net_config::test_macros::basic_tests;
    use crate::net_config::test_macros::dhcp_tests;

    // The macros take a version as their only parameter
    basic_tests!(1);
    dhcp_tests!(1);
...
}

The output includes a single line for each version and test. It would look like the following for a v1 and (not yet implemented) v2.

test net_config::v1::tests::dhcp::invalid_dhcp_config ... ok
test net_config::v1::tests::dhcp::invalid_dhcp6_options ... ok
test net_config::v1::tests::basic::multiple_primary_interfaces ... ok
test net_config::v1::tests::basic::undefined_primary_interface ... ok
test net_config::v2::tests::dhcp::invalid_dhcp_config ... ok
test net_config::v2::tests::dhcp::invalid_dhcp6_options ... ok
test net_config::v2::tests::basic::multiple_primary_interfaces ... ok
test net_config::v2::tests::basic::undefined_primary_interface ... ok

Commit messages:

    netdog: Add macros to simplify and improve net config testing
    
    This commit adds a new module `test_macros` inside of `net_config`.  The
    module is meant to contain a group of declarative macros, each of which
    contain a scoped group of tests and can be used across net config
    versions.  The macros themselves are declarative and define a new module
    that contains all the tests.  This allows us to use the same tests
    across all versions of net config, and get nicely formatted `cargo test`
    output showing the net config version, module, and test.  This is much
    nicer than looping over versions within a single test, and provides much
    better error output for the user
    netdog: Update net_config to use test macros
    
    This change updates `net_config` to take advantage of the previously
    added test macros.  It removes the majority of the tests from `mod.rs`,
    since the test themselves were moved to macros.  It updates the location
    of the test data files, and templates the version.  The last and final
    change is to add the test macros to `v1`.

Testing done:

  • Ensure all the unit tests continue to pass
 $ cargo test
    Finished test [unoptimized + debuginfo] target(s) in 0.09s
     Running unittests src/main.rs (/home/fedora/bottlerocket/bottlerocket/sources/target/debug/deps/netdog-a2a46d1268055d1c)

running 24 tests
test cli::prepare_primary_interface::tests::default_sysctls ... ok
test net_config::tests::multiple_interface_from_cmdline ... ok
test interface_name::tests::valid_interface_name ... ok
test interface_name::tests::invalid_interface_name ... ok
test net_config::tests::no_interfaces_cmdline ... ok
test net_config::tests::no_interfaces_net_config ... ok
test net_config::tests::ok_cmdline ... ok
test net_config::v1::tests::basic::no_interfaces ... ok
test net_config::v1::tests::basic::invalid_interface_config ... ok
test net_config::v1::tests::dhcp::dhcp4_missing_enable ... ok
test net_config::v1::tests::dhcp::invalid_dhcp4_options ... ok
test net_config::v1::tests::invalid_interface_from_str ... ok
test net_config::v1::tests::dhcp::dhcp6_missing_enable ... ok
test net_config::v1::tests::basic::invalid_version ... ok
test net_config::v1::tests::ok_interface_from_str ... ok
test net_config::v1::tests::dhcp::invalid_dhcp_config ... ok
test net_config::v1::tests::dhcp::invalid_dhcp6_options ... ok
test net_config::v1::tests::basic::multiple_primary_interfaces ... ok
test net_config::v1::tests::basic::undefined_primary_interface ... ok
test net_config::tests::ok_net_config ... ok
test net_config::v1::tests::basic::defined_primary_interface ... ok
test net_config::v1::tests::basic::ok_config ... ok
test wicked::tests::interface_config_from_str ... ok
test wicked::tests::net_config_to_interface_config ... ok

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@zmrow zmrow requested review from bcressey, cbgbt and etungsten August 29, 2022 18:47
@etungsten etungsten removed the request for review from cbgbt August 30, 2022 15:58
Copy link
Contributor

@etungsten etungsten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌮

Copy link
Member

@markusboehme markusboehme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The choice of macros here is an interesting one, but seems to fit the bill! Definitely passes my Rust newb readability check. 🥇

This commit adds a new module `test_macros` inside of `net_config`.  The
module is meant to contain a group of declarative macros, each of which
contain a scoped group of tests and can be used across net config
versions.  The macros themselves are declarative and define a new module
that contains all the tests.  This allows us to use the same tests
across all versions of net config, and get nicely formatted `cargo test`
output showing the net config version, module, and test.  This is much
nicer than looping over versions within a single test, and provides much
better error output for the user.
This change updates `net_config` to take advantage of the previously
added test macros.  It removes the majority of the tests from `mod.rs`,
since the test themselves were moved to macros.  It updates the location
of the test data files, and templates the version.  The last and final
change is to add the test macros to `v1`.
@zmrow
Copy link
Contributor Author

zmrow commented Sep 1, 2022

^ Per @bcressey, moves to handlebars as the template renderer

@zmrow zmrow merged commit e4ebf23 into bottlerocket-os:develop Sep 1, 2022
@zmrow zmrow deleted the test-macros branch September 1, 2022 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants