use crate::code_lint::ast::{self, ParsedFile};
use crate::code_lint::contract::{CodeRule, RuleTarget};
use crate::diagnostic::{Diagnostic, Language, RuleName, ViolationTemplate, violation_template};
use crate::rule_declaration::{
Classification, Consensus, Declaration, Example, ImpactedQuality, Precision, Reference,
RuleDoc, RuleOptions, Topic,
};
use std::path::Path;
const TEMPLATE: ViolationTemplate = violation_template! {
summary: "Test `{function}` interleaves actions and assertions across multiple phases.",
rationale: "Alternating between actions and assertions in one test couples multiple scenarios together, so an early failure skips later phases and obscures which state transition broke.",
suggestion: {
Python => "Split each arrange-act-assert phase into its own `test_*` function, or move all setup actions above the assertions.",
Rust => "Split each arrange-act-assert phase into its own `#[test]` function, or move all setup actions above the assertions.",
},
};
pub const RULE: CodeRule = CodeRule {
declaration: Declaration {
name: RuleName("multi-phase-test"),
template: &TEMPLATE,
languages: &[Language::Python, Language::Rust],
options: RuleOptions::code_rule(()),
classification: Classification {
topics: &[Topic::TEST_ASSERTIONS],
precision: Precision::Heuristic,
consensus: Consensus::Opinionated,
impacted_quality: ImpactedQuality::Maintainability,
},
doc: RuleDoc {
summary: "Flags test functions that perform further actions and assertions after an earlier assertion.",
what_it_does: indoc::indoc! {r"
Flags a test function whose top-level statements follow an assert-act-assert
pattern: one or more assertions (including `assert_*` / `_assert_*` calls and calls
to same-file assertion helpers), followed by a non-assertion statement, followed by
another assertion.
Call-free local variable projections (`order = result.orders[0]`,
`let order = &result.orders[0];`), `pytest` capture readouts
(`captured = capsys.readouterr()`), and Python `assert x is not None` or
`assert isinstance(x, ...)` narrowing guards (when `x` is read later in the test)
are treated as part of the assertion phase."},
why_is_this_bad: indoc::indoc! {r"
A test that mutates state, asserts, mutates state again, and asserts a second time
is testing a sequence of scenarios inside a single function. When the first phase
fails, the second phase never runs, and when the second phase fails, the reader has
to trace the intermediate state mutations from the first phase to understand the
preconditions.
Give each behavior its own arrange-act-assert test function, or compute the
values under test before running the assertions."},
known_problems: Some(indoc::indoc! {r"
- Stateful lifecycle or protocol handshake tests that intentionally step through a
sequence of transitions in one test need an `# omni:ignore[multi-phase-test]`
suppression with a reason.
- Imported helper calls that perform assertions without an `assert_` or `_assert_`
prefix are classified as non-assertion statements."}),
references: &[Reference {
title: "Software Engineering at Google, ch. 12: Structure Tests to Emphasize Behaviors",
url: "https://abseil.io/resources/swe-book/html/ch12.html",
}],
examples: &[
Example {
language: Language::Python,
flagged: indoc::indoc! {r#"
def test_cart_lifecycle():
cart = Cart()
cart.add("book")
assert len(cart.items) == 1
cart.clear()
assert len(cart.items) == 0
"#},
flagged_span: "test_cart_lifecycle",
fixed: indoc::indoc! {r#"
def test_cart_add_item():
cart = Cart()
cart.add("book")
assert len(cart.items) == 1
def test_cart_clear():
cart = Cart(items=["book"])
cart.clear()
assert len(cart.items) == 0
"#},
},
Example {
language: Language::Rust,
flagged: indoc::indoc! {r#"
#[test]
fn test_cart_lifecycle() {
let mut cart = Cart::new();
cart.add("book");
assert_eq!(cart.len(), 1);
cart.clear();
assert_eq!(cart.len(), 0);
}
"#},
flagged_span: "test_cart_lifecycle",
fixed: indoc::indoc! {r#"
#[test]
fn test_cart_add_item() {
let mut cart = Cart::new();
cart.add("book");
assert_eq!(cart.len(), 1);
}
#[test]
fn test_cart_clear() {
let mut cart = Cart::from_items(["book"]);
cart.clear();
assert_eq!(cart.len(), 0);
}
"#},
},
],
},
},
target: RuleTarget::TestsOnly,
check: check_file,
};
fn check_file(rule: &CodeRule, path: &Path, file: &ParsedFile, (): ()) -> Vec<Diagnostic> {
ast::collect_multi_phase_test_functions(file)
.into_iter()
.map(|test_fn| {
rule.diagnostic_at_node(path, &test_fn.name_node, &[("function", &test_fn.name)])
})
.collect()
}
#[cfg(test)]
crate::test_utils::rule_test!(
RULE,
{
Python => {
pass: [
helper_function_exempt => r#"
def verify_transitions(cart):
cart.add("book")
assert len(cart.items) == 1
cart.clear()
assert len(cart.items) == 0
"#,
single_phase_multiple_assertions_allowed => r#"
def test_parse_header():
header = parse_header("X-Request-Id: 42")
assert header.name == "X-Request-Id"
assert header.value == "42"
"#,
narrowing_is_not_none_before_action_exempt => r#"
def test_narrowed_user_profile(repo):
user = repo.find_by_id("u1")
assert user is not None
profile = user.load_profile()
assert profile.is_active
"#,
narrowing_isinstance_between_assertions_exempt => r#"
def test_narrowed_event_payload(bus):
event = bus.pop()
assert event.topic == "orders"
assert isinstance(event, OrderCreated)
assert event.order_id == 7
"#,
call_free_projection_binding_between_assertions_exempt => r"
def test_batch_order_fields():
batch = process_batch()
assert len(batch.orders) == 1
order = batch.orders[0]
assert order.price == 10
assert order.size == 5
",
capsys_readouterr_after_pytest_raises_exempt => r#"
import pytest
def test_cli_invalid_wallet_exits(capsys):
with pytest.raises(SystemExit) as exc_info:
run_cli("--wallet", "invalid")
captured = capsys.readouterr()
assert exc_info.value.code == 1
assert "invalid wallet" in captured.err
"#,
nested_function_assertions_not_counted => r"
def test_with_local_verifier():
def check_positive(val):
assert val > 0
result = compute_score()
assert result == 10
",
],
fail: [
act_between_assert_statements => r#"
def test_queue_push_pop():
queue = Queue()
queue.push("first")
assert queue.size() == 1
popped = queue.pop()
assert popped == "first"
"# => "test_queue_push_pop",
act_after_pytest_raises_block => r#"
import pytest
def test_error_then_recovery(service):
with pytest.raises(ValueError):
service.execute("invalid")
result = service.execute("valid")
assert result == "ok"
"# => "test_error_then_recovery",
act_between_unittest_assertions => r"
class OrderTest:
def test_submit_then_cancel(self):
order = Order()
self.assertTrue(order.is_draft())
order.cancel()
self.assertTrue(order.is_cancelled())
" => "test_submit_then_cancel",
unreferenced_is_not_none_counts_as_assertion => r#"
def test_two_independent_queries(repo):
first = repo.find("a")
assert first is not None
second = repo.find("b")
assert second == "b"
"# => "test_two_independent_queries",
attribute_mutation_between_assertions => r#"
def test_mutate_attribute_between_assertions():
order = Order(status="open")
assert order.status == "open"
order.status = "cancelled"
assert order.status == "cancelled"
"# => "test_mutate_attribute_between_assertions",
act_between_same_file_assertion_helper_calls => r#"
def _verify_cart_len(cart, expected):
assert len(cart.items) == expected
def test_cart_transitions():
cart = Cart()
cart.add("book")
_verify_cart_len(cart, 1)
cart.clear()
_verify_cart_len(cart, 0)
"# => "test_cart_transitions",
],
},
Rust => {
pass: [
helper_function_exempt => r#"
fn verify_transitions(cart: &mut Cart) {
cart.add("book");
assert_eq!(cart.len(), 1);
cart.clear();
assert_eq!(cart.len(), 0);
}
"#,
single_phase_multiple_assertions_allowed => r#"
#[test]
fn test_parse_header() {
let header = parse_header("X-Request-Id: 42");
assert_eq!(header.name, "X-Request-Id");
assert_eq!(header.value, "42");
}
"#,
local_item_declaration_between_assertions_exempt => r"
#[test]
fn test_with_local_type() {
assert_eq!(1 + 1, 2);
struct Marker;
assert_eq!(2 + 2, 4);
}
",
call_free_projection_binding_between_assertions_exempt => r"
#[test]
fn test_batch_order_fields() {
let batch = process_batch();
assert_eq!(batch.orders.len(), 1);
let order = &batch.orders[0];
assert_eq!(order.price, 10);
assert_eq!(order.size, 5);
}
",
],
fail: [
method_call_between_assertions => r"
#[test]
fn test_stack_push_pop() {
let mut stack = Vec::new();
stack.push(10);
assert_eq!(stack.len(), 1);
stack.pop();
assert!(stack.is_empty());
}
" => "test_stack_push_pop",
let_binding_between_assertions => r"
#[test]
fn test_two_independent_calculations() {
let first = compute(1);
assert_eq!(first, 2);
let second = compute(2);
assert_eq!(second, 4);
}
" => "test_two_independent_calculations",
tail_expression_assertion_after_action => r"
#[test]
fn test_tail_assertion() {
let mut count = 1;
assert_eq!(count, 1);
count += 1;
assert_eq!(count, 2)
}
" => "test_tail_assertion",
act_between_same_file_assertion_helper_calls => r#"
fn verify_cart_len(cart: &Cart, expected: usize) {
assert_eq!(cart.len(), expected);
}
#[test]
fn test_cart_transitions() {
let mut cart = Cart::new();
cart.add("book");
verify_cart_len(&cart, 1);
cart.clear();
verify_cart_len(&cart, 0);
}
"# => "test_cart_transitions",
],
}
}
);