Test Anti-Patterns: The Run-on Test

Mastering automated testing is much more challenging than people realize at first. Internet code samples are naively simple and often don’t address the issues of getting tests into place for legacy code. It can be really hard to get started with automated testing, and doing it badly can easily sour one’s judgment on the overall value. This post is the first in a series that will identify some testing anti-patterns to help you understand if you’re automated testing efforts are going down a bad path. I’ve seen these in the wild many times. Usually these test anti-patterns emerge when people do "Test After Development" (as opposed to "Test Driven Development"), or when people are new to automated testing.

Recall the guidance I wrote about Good unit tests.

In general, automated tests should be structured in a rigorous way:

#[test]
pub fn my_test() {
    // Given
    let context = SomeTestContextThatINeed();
    context.setup_with_data_and_or_expectations();

    // When
    let sut = SystemUnderTest {};
    let result = sut.do_the_action_under_test();

    // Then
    assert_eq!(result, Ok(expected_value));
}

  • The Given section should include any context setting required by the test.
  • The When section should include the operation under test. We should strive for a one-liner here as it makes it obvious at a glance what is being tested.
  • The Then section should contain the various checks on expectations.

(Some authors use a different mnemonic to represent the same thing: "Arrange, Act, Assert". This verbiage is identical to "Given, When, Then" in intent. I have come to prefer "Given, When, Then" because it’s in more common usage with project managers and is therefore less domain language to maintain.)

The Structure of a Run-On Test

Here is an example of what’s called a "run-on" test. They are often written by engineers new to unit testing or by engineers who are writing tests after the fact as opposed to practicing TDD:

#[test]
pub fn my_test() {
    // Given
    let context = SomeTestContextThatINeed();
    context.setup_with_data_and_or_expectations();

    // When
    let sut = SystemUnderTest {};
    let result = sut.do_the_action_under_test();

    // Then
    assert_eq!(result, Ok(expected_value));

    // Given
    context.alter_the_state_in_some_way();

    // When
    let new_result = sut.do_some_other_action();

    // Then
    assert_eq!(new_result, Ok(some_other_expected_value));
}

The "run-on test" follows the Then section with additional actions that require verification. This is problematic because now the test can fail for more than one reason.

A Run-On Test differs from a test with multiple assert statements. There is a separate argument about whether you should or shouldn’t have multiple assert_eq!() statements in a test body. That’s an interesting and legitimate debate, but is a conceptually distinct question from run-on tests. While these are different issues, they are often discussed together. Personally I think multiple asserts are fine so long as there are not multiple When sections in the test. In other words, feel free to assert on all the side-effects of a single action.

Question: What if we called sut.do_the_action_under_test() again instaed of sut.do_some_other_action()?

If you have altered the setup or parameters in any way then the test can fail for more than one reason.

Question: Won’t this lead to a lot of duplicated test code?

Possibly. However, you should treat your test code with the same care toward readability as your production code. Extract shared code into reusable, composable components just as you would any other code. Even if there is duplication, remember the tests are a specification so duplication between tests is not nearly as important as making the requirements clearly understood.

A More Realistic Example of a Run-On Test

Let’s imagine an engineer who writes a test after implementing some auth middlware.

#[actix_rt::test]
async fn auth_works() {
    let cert = Some("some special cert".to_string());
    let mut app = test::init_service(
        App::new()
            .wrap(crate::middleware::auth::Auth { cert })
            .configure(|c| {
                c.service(home::routes());
            }),
    )
    .await;

    let req = test::TestRequest::get().uri("/").to_request();

    let resp = test::call_service(&mut app, req).await;

    assert!(resp.stastus().is_client_error())
    assert_eq!(resp.status(), StatusCode::UNAUTHORIZED);

    let req = test::TestRequest::get()
        .header("Authorization", cert)
        .uri("/")
        .to_request();
    let resp = test::call_service(&mut app, req).await;

    assert!(resp.status().is_success());
}

Why is this a problem?

What is the output if assert!(resp.stastus().is_client_error()) fails? Something like:

auth works failed. Expected: true Actual: false

Can you tell what requiremnt is broken from that? Even scanning the test can you tell? Can we say that a test like this is functioning as a specification? No.

There are two different assertions in the test that could lead to this output:

assert!(resp.stastus().is_client_error())
...
assert!(resp.status().is_success());

If the first assertion fails, the rest of the test does not execute. Now you don’t know if it’s that one specification that’s broken or if any of the other specs are broken too.

When you encounter this kind of test in the wild, it will likely have many more sections and assertions making it even harder to interpret than what we have here.

Recall Scott Bain’s advice about Good Unit Tests

  1. The test must fail reliably for the reason intended.
  2. The test must never fail for any other reason.
  3. There must be no other test that fails for this reason.

The test above fails 2/3 of the criteria listed here.

Question: Who cares? What is the value to be achieved with those criteria?

If the test fails reliabily for the reason intended, encountering the test failure will guide you to the problem.

If the test fails for any reason but the intended, then time spent tracking down and repairing the problem will increase dramatically.

This is a case where a little discipline goes a long way.

After Refactoring

#[actix_rt::test]
async fn auth_not_specified() {
    let cert = Some("some special cert".to_string());
    let mut app = create_web_app_using_cert(cert).await;

    let req = test::TestRequest::get().uri("/").to_request();

    let resp = test::call_service(&mut app, req).await;

    assert!(resp.stastus().is_client_error())
    assert_eq!(resp.status(), StatusCode::UNAUTHORIZED);
}

#[actix_rt::test]
async fn auth_provided_and_valid() {
    let cert = Some("some special cert".to_string());
    let mut app = create_web_app_using_cert(cert).await;

    let req = test::TestRequest::get()
        .header("Authorization", cert)
        .uri("/")
        .to_request();
    let resp = test::call_service(&mut app, req).await;

    assert!(resp.status().is_success());
}

#[actix_rt::test]
async fn auth_provided_but_invalid() {
    let cert = Some("some special cert".to_string());
    let mut app = create_web_app_using_cert(cert).await;

    let req = test::TestRequest::get()
        .header("Authorization", "some random invalid cert")
        .uri("/")
        .to_request();
    let resp = test::call_service(&mut app, req).await;

    assert!(resp.status().is_client_error())
    assert_eq!(resp.status(), StatusCode::UNAUTHORIZED);
}

Notice also how these tests clarify the specification that must be met. This is a critical point. Tests help tell the story of the code.

Did you notice the missing test in the original sample? We completely missed testing invalid auth.

Given that these test methods are well-named and small, it was easy to see that a test case is missing during the refactoring. This is one of the dangers of "Test After" as opposed to TDD: you are likely to omit important test cases. If you are disciplined about writing tests first, that won’t happen because you can’t write the production code without having a failing test in place first.

Further Reading

Kent Beck wrote a nice summary of the issues with run-on tests and what to do about them here.

Multiple Asserts are OK

Avoid Multiple Asserts in a Single Unit Test

Leave a Reply

%d bloggers like this: