refactor(backend-2fa): split the 6,300-line test module into discoverable suites - #1277
Open
rudeus112266 wants to merge 1 commit into
Open
refactor(backend-2fa): split the 6,300-line test module into discoverable suites#1277rudeus112266 wants to merge 1 commit into
rudeus112266 wants to merge 1 commit into
Conversation
The 6,363-line tests.rs was a single flat file mixing enrollment,
verification, recovery, admin, rate-limit, and integration tests plus
~15 ad-hoc nested test modules, making it hard to navigate or assign.
Split into backend-2fa/src/tests/{enrollment,verification,recovery,
rate_limit,admin,integration}.rs by domain, with shared test fixtures
(caller, generate_token, admin, MockRedisState, etc.) hoisted into a
common.rs used by all six. Pure reorganization: every one of the 260
original #[test]/#[tokio::test]/#[actix_web::test] functions is
preserved verbatim in exactly one file, with no assertion or behavior
changes, verified by exact test-count diff before/after.
One incidental fix: test_tenant_scoped_disable_does_not_affect_other_tenant
was missing a .clone() on a reused token, a pre-existing bug in the
original file that made the whole crate fail to compile as tests --
fixed narrowly so the split file, and the tests within it, actually
build.
|
@rudeus112266 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
backend-2fa/src/tests.rswas a single 6,363-line file mixing enrollment, verification, recovery, admin, rate-limiting, and integration tests together (plus ~15 ad-hoc nested test modules with inconsistent boundaries), making it hard to navigate, parallelize, or assign ownership of.Split it into
backend-2fa/src/tests/{enrollment,verification,recovery,rate_limit,admin,integration}.rs, organized by domain as requested. Shared test fixtures that were previously duplicated across several originally-separate nested modules (caller,generate_token,admin,MockRedisState,mock_record_failure,RecordingHttpClient, etc.) are hoisted once into a newtests/common.rs, glob-imported by all six domain files.Scope
Pure structural reorganization — no test logic, assertions, or production code changed. Domain classification was done with a script (name/section-based, cross-checked against the original file's own section comments) and mechanically verified: every one of the 260 original
#[test]/#[tokio::test]/#[actix_web::test]functions is preserved in exactly one output file — counted before and after and confirmed to match exactly, so nothing was lost, duplicated, or silently dropped.One incidental fix (disclosed)
While verifying the split actually compiles, I found
test_tenant_scoped_disable_does_not_affect_other_tenantreuses a movedtoken_awithout.clone()— a pre-existing bug in the originaltests.rs(verified byte-identical ingit show upstream/main:backend-2fa/src/tests.rs, not introduced by this split) that blocks the whole crate from compiling in test configuration. Fixed with a single.clone()since otherwise this PR's own claim of "tests still compile" couldn't be verified at all. No other test bodies were touched.Testing
cargo test -p petchain-2fa --lib --no-run: after the split, only the same pre-existing, unrelated compile errors remain as onupstream/mainbefore this change (missingenroll_lockfield onTwoFactorHandlers,TenantScopedStorenot implementingTwoFactorStore, a missingtest_two_factor_storehelper) — confirmed by diffing the error set before/after; this PR introduces zero new compile errors.upstream/maindoes not currently compile its test target at all (independent of this PR), so "all tests pass" isn't achievable until that's fixed separately.rustfmt --checkon all 8 new/changed files: clean.cargo clippy --all-targets: halts on the same pre-existing lib errors asupstream/main, so no new clippy findings could be introduced by this diff either.grep -c '#\[test\]\|#\[tokio::test\]\|#\[actix_web::test\]'on the original file vs. the sum across all 6 new files: 260 = 260.Threat model
None — this is a test-only file reorganization with no production code, storage, ABI, or authorization changes.
Closes #1219