Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dwctl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ google-cloud-storage = "1.12"
google-cloud-auth = "1.10"

[dev-dependencies]
axum-test = { version = "18.4.1" }
axum-test = { version = "20.0.0" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: Major version upgrade from 18.4.1 to 20.0.0 (skipping v19).

Why it matters: Major version bumps typically indicate breaking changes. While the axum-test v20 documentation shows the same core APIs this codebase uses (TestServer::new, multipart::Part, MultipartForm), there could be signature changes, trait bound modifications, or behavioral differences not visible from documentation alone. Skipping v19 entirely is unusual and warrants checking the changelog to understand what changed.

Suggested fix: Verify locally that cargo build and cargo test succeed with this version before merging. If possible, check the axum-test GitHub repository changelog to confirm v19 was intentionally skipped and understand what breaking changes were introduced in v19/v20.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This update to axum-test v20.0.0 requires axum v0.8.8+ according to the official compatibility table, but the current axum dependency is declared as version = "0.8" which could resolve to earlier 0.8.x versions.

Why it matters: If Cargo resolves axum to a version < 0.8.8 (e.g., in a clean build or CI environment with cached lock files), there may be API incompatibilities between axum and axum-test that cause compilation failures or subtle runtime issues in tests. The axum-test library tightly couples to specific axum versions because it needs to understand axum's internal service types.

Suggested fix: Update the axum dependency to explicitly require v0.8.8+:

axum = { version = "0.8.8", features = ["multipart"] }

This ensures both the production dependency and test dependency are aligned with their mutual compatibility requirements.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: Major version upgrade from 18.4.1 to 20.0.0 introduces breaking API changes that require code modifications.

Why it matters: In axum-test v19.0.0, TestServer::new() was changed to no longer return a Result - it now panics on failure or returns TestServer directly. The current codebase has ~46 instances of TestServer::new(...).expect(...) or .unwrap() that will fail to compile with this upgrade because you can't call .expect() on a non-Result type.

Per the v19.0.0 release notes: "new() constructor no longer returns a Result, and instead panics on failure. Added try_new() to allow people to continue to catch the Result if needed."

Suggested fix: Either:

  1. Remove .expect()/.unwrap() from all TestServer::new() calls (simplest)
  2. Replace with TestServer::try_new() if explicit error handling is preferred

Example change:

// Before (v18.x)
let server = TestServer::new(router).expect("Failed to create test server");

// After (v19+)
let server = TestServer::new(router);
// OR
let server = TestServer::try_new(router).expect("Failed to create test server");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: Major version upgrade from v18.4.1 to v20.0.0 requires code changes.

Why it matters: axum-test v19 introduced breaking changes that affect how TestServer::new() is used. The old API returned Result<TestServer, Error> requiring .expect() or ?, but v19+ changed to direct panic on failure with a new try_new() method for Result-based handling.

The codebase has 11 instances of TestServer::new(...).expect("Failed to create test server") across:

  • src/test/mod.rs (4 instances)
  • src/lib.rs (1 instance)
  • src/error_enrichment.rs (6 instances)

While these will still compile (due to Into trait conversions), the pattern is now semantically incorrect.

Suggested fix: Either:

  1. Remove .expect() calls: let server = TestServer::new(router);
  2. Or use new API: let server = TestServer::try_new(router)?; (if you want explicit error handling)

Additionally, run the full test suite to verify behavioral changes in the mocked transport layer don't break any tests.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: axum-test v20.0.0 requires axum 0.8.8+ according to the axum-test compatibility table, but the axum dependency on line 21 is specified as version = "0.8" which could resolve to versions lower than 0.8.8.

Why it matters: If Cargo resolves axum to a version below 0.8.8 (e.g., 0.8.0-0.8.7), the build will fail due to version incompatibility between axum and axum-test. This creates an unstable build that depends on what version Cargo happens to resolve.

Suggested fix: Update the axum dependency on line 21 to specify the minimum compatible version:

axum = { version = "0.8.8", features = ["multipart"] }

Alternatively, if you want to keep the broader 0.8 range, you should pin axum-test to v19.0.0 instead, which supports axum 0.8.8+ but may work with earlier 0.8.x versions.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: Major version upgrade from 18.x to 20.x warrants test verification.

Why it matters: Major version bumps typically include breaking changes. While the core TestServer::new() API appears stable per docs.rs documentation, the skip from v18 directly to v20 (no v19 release visible) suggests possible significant changes. The codebase has ~50 usages of TestServer::new() across test files that depend on this library's API remaining compatible.

Suggested fix: Run cargo test --package dwctl to verify all tests pass with the new version before merging. Additionally, check the axum-test changelog at https://github.com/prestontules/axum-test/releases for any breaking changes that might require test code updates.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This major version upgrade (v18 β†’ v20) introduces a breaking API change that will cause 57 compilation errors in the test suite.

Why it matters: In v18.4.1, TestServer::new() returns Result<Self>, but in v20.0.0 it returns Self directly and panics on failure. The codebase uses .expect() and .unwrap() on the return value throughout test files, which will fail to compile because you cannot call these methods on a non-Result type.

Suggested fix: Either (1) update all call sites to remove .expect()/.unwrap() calls, or (2) migrate to the new try_new() method for explicit error handling. Run cargo check to identify all affected locations.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This version upgrade introduces a breaking API change that will cause compilation failures.

Why it matters: In axum-test v19+, TestServer::new() returns TestServer directly instead of Result<TestServer, E>. The codebase has 11 occurrences of .expect() being called on the result, which will fail to compile because you cannot call .expect() on a non-Result type.

Suggested fix: Alongside this dependency update, remove .expect("Failed to create test server") from all TestServer::new() calls. For example, change:

let server = axum_test::TestServer::new(service).expect("Failed to create test server");

to:

let server = axum_test::TestServer::new(service);

If you need explicit error handling, use try_new() instead which still returns a Result.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: Major version upgrade from v18 to v20 introduces breaking API changes.

Why it matters: In axum-test v19+, TestServer::new() no longer returns a Result<T, E> - it panics directly on failure. The current code has 11 instances of .expect("Failed to create test server") chained after TestServer::new(), which will fail to compile because you cannot call .expect() on a non-Result type.

Per the v19.0.0 release notes:

new() constructor no longer returns a Result, and instead panics on failure.
Added try_new() to allow people to continue to catch the Result if needed.

Suggested fix: Either:

  1. Remove the .expect() calls since panic-on-failure is now built-in:

    // Before
    let server = axum_test::TestServer::new(router).expect("Failed to create test server");
    
    // After
    let server = axum_test::TestServer::new(router);
  2. Or use the new try_new() if you need explicit error handling:

    let server = axum_test::TestServer::try_new(router).expect("Failed to create test server");

Affected files with this pattern:

  • dwctl/src/lib.rs:2890
  • dwctl/src/error_enrichment.rs (6 locations)
  • dwctl/src/test/mod.rs (4 locations)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This upgrade from v18.4.1 to v20.0.0 introduces a breaking API change that affects 46+ test locations in this codebase.

Why it matters: In axum-test v19.0.0 (and therefore v20.0.0), TestServer::new() was changed to no longer return a Result - it now panics on failure directly. The existing code pattern TestServer::new(app).unwrap() will fail to compile because .unwrap() cannot be called on a non-Result type.

From the v19.0.0 release notes:

  • new() constructor no longer returns a Result, and instead panics on failure.
  • Added try_new() to allow people to continue to catch the Result if needed.

Suggested fix: Replace all instances of:

let server = TestServer::new(app).unwrap();

With:

let server = TestServer::new(app);

Alternatively, if you need explicit error handling in tests, use TestServer::try_new(app)? and ensure test functions return Result<(), Box<dyn std::error::Error>>.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: Major version dependency update requires verification.

Why it matters: Upgrading from v18.4.1 to v20.0.0 spans two major versions (v19 and v20), which typically indicates breaking changes according to semver. While the core API surface (TestServer::new, multipart module) appears compatible based on docs.rs documentation, the official changelog was inaccessible during review and compilation cannot be verified in this environment.

Suggested fix: Before merging:

  1. Run cargo test locally to verify all tests pass
  2. Review the GitHub releases for v19.0.0 and v20.0.0 to identify any breaking changes affecting your usage patterns
  3. Pay special attention to multipart form handling since ~40 test cases use axum_test::multipart::Part and MultipartForm

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: The corresponding Cargo.lock file has not been updated to reflect this version change.

Why it matters: When Cargo.lock exists, Cargo uses the locked versions rather than resolving fresh from Cargo.toml. The lockfile currently pins axum-test at v18.7.0, so this change to v20.0.0 won't take effect until the lockfile is regenerated. This means:

  • CI/CD will continue testing against v18.7.0
  • Developers won't actually get v20.0.0 when pulling this branch
  • The dependency update is effectively inert without the lockfile change

Suggested fix: Run cargo update -p axum-test in the dwctl/ directory to update Cargo.lock, then commit those changes as part of this PR. This ensures the version bump is actually applied.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This upgrade introduces a breaking API change that will cause ~56 compilation errors across the test suite.

Why it matters: In axum-test v19.0.0+, TestServer::new() was changed to return Self directly instead of Result<Self, E>. It now panics internally on failure. The codebase currently uses patterns like:

let server = axum_test::TestServer::new(router).expect("Failed to create test server");

This will fail to compile because .expect() doesn't exist on TestServer.

Per the v20 docs.rs: "Note: this will panic if the TestServer cannot be built. To catch the error use TestServer::try_new."

Suggested fix: Remove all .expect() and .unwrap() calls from TestServer::new() invocations. For example:

// Before
let server = axum_test::TestServer::new(router).expect("Failed to create test server");

// After
let server = axum_test::TestServer::new(router);

Alternatively, use TestServer::try_new() if explicit error handling is needed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: Consider pinning to a more specific version range or adding a comment noting why this major version was chosen.

Why it matters: Major version bumps in dependencies can introduce breaking changes. While the axum-test v20 API appears compatible with current usage patterns based on docs.rs documentation, having explicit version constraints helps future maintainers understand the intent and makes debugging easier if issues arise.

Suggested fix: Either:

  1. Add a comment explaining the upgrade rationale (e.g., # Updated to v20 for axum 0.8 compatibility)
  2. Or use a more specific semver range like "20.0" to automatically pick up patch fixes while staying within the tested minor version

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This version upgrade introduces a breaking API change that will cause 56 compilation failures.

Why it matters: In axum-test v20, TestServer::new() returns Self directly instead of Result<Self>. All existing calls using .expect() or .unwrap() on the return value will fail to compile with error E0599 (no method named unwrap found).

Suggested fix: Either:

  1. Update all 56 call sites to remove .expect()/.unwrap(): TestServer::new(app)
  2. Or use TestServer::try_new(app).expect("...") if you want to preserve explicit error handling (though new() is now infallible by design)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This major version upgrade (v18 β†’ v20) introduces a breaking API change that will cause compilation failures.

Why it matters: In axum-test v19.0.0+, TestServer::new() returns TestServer directly instead of Result<TestServer, Error>. The codebase has 56 usages of .expect() or .unwrap() on the result of TestServer::new(), which will fail to compile because TestServer doesn't implement these methods.

From the v19.0.0 release notes: "new() constructor no longer returns a Result, and instead panics on failure. Added try_new() to allow people to continue to catch the Result if needed."

Suggested fix: Either:

  1. Keep version at 18.4.1 until test code can be updated
  2. Or update all usages to remove .expect()/.unwrap() calls (since v19+ panics internally), or use try_new() for error-handling variants:
    // Old (v18):
    let server = TestServer::new(app).expect("...");
    
    // New (v19+):
    let server = TestServer::new(app);  // panics internally
    // OR:
    let server = TestServer::try_new(app)?;  // returns Result

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This major version upgrade introduces breaking changes that will cause compilation failures.

Why it matters: In axum-test v19.0.0+, TestServer::new() no longer returns a Result - it panics directly on failure. The new try_new() method was added for Result-based error handling. Since this codebase has 56 usages of TestServer::new(...).unwrap() or TestServer::new(...).expect(...), all of them will fail to compile with errors like:

error[E0599]: no method named `unwrap` found for struct `TestServer` in the current scope

According to the v19.0.0 release notes:

new() constructor no longer returns a Result, and instead panics on failure.
Added try_new() to allow people to continue to catch the Result if needed.

Suggested fix: Either:

  1. Update all 56 call sites to remove .unwrap()/.expect() chains
  2. Replace TestServer::new(...) with TestServer::try_new(...) where you want to preserve explicit error handling
  3. Consider bumping to v20.1.0 (latest) which may have additional fixes

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: Major version upgrade with breaking API change.

Why it matters: In axum-test v18, TestServer::new() returned Result<TestServer> and could fail. In v20, it returns TestServer directly and panics on failure. A new try_new() method provides the fallible alternative.

This affects 56 test locations across the codebase that call TestServer::new(). While .unwrap() and .expect() calls will still compile, the semantics have changed - you're now calling a method that panics rather than handling a Result.

Suggested fix: Decide on your error handling strategy:

  1. Accept panicking (recommended for most tests): Keep using new() as-is. Panics are acceptable in test setup.

  2. Custom error messages: For tests where failure context matters, migrate to:

    let server = TestServer::try_new(app).expect("Failed to create test server")?;

See docs.rs for v18.4.1 vs v20.0.0 for the API difference.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This major version upgrade (18β†’20) introduces a breaking API change that will cause compilation failures.

Why it matters: In axum-test v18.4.1, TestServer::new() returns Result<Self>, but in v20.0.0 it returns Self directly and panics on failure (a new try_new() method was added for error handling). The codebase has 56 occurrences of TestServer::new(...).expect(...) or TestServer::new(...).unwrap() that will fail to compile because you cannot call .expect() or .unwrap() on a non-Result type.

All affected files:

  • dwctl/src/test/mod.rs (3 occurrences)
  • dwctl/src/lib.rs (1 occurrence)
  • dwctl/src/error_enrichment.rs (6 occurrences)
  • dwctl/src/api/handlers/payments.rs (19 occurrences)
  • dwctl/src/api/handlers/auth.rs (27 occurrences)

Suggested fix: Remove the .expect() and .unwrap() calls from all TestServer::new() invocations. For example:

// Before:
let server = TestServer::new(app).expect("Failed to create test server");

// After:
let server = TestServer::new(app);

If any tests need custom error handling, use TestServer::try_new(app)? instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: Major version upgrade from 18.4.1 to 20.0.0. This skips v19 entirely and may include breaking API changes.

Why it matters: The codebase has ~885 usages of axum-test APIs including TestServer::new(), assert_status_*() methods, and request builders. Any breaking changes would require updates across many test files.

Suggested fix: Verify this compiles and all tests pass by running just test rust locally before merging. If tests fail, check the axum-test GitHub repository for migration guides or consider upgrading incrementally (v18 β†’ v19 β†’ v20) to isolate any breaking changes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This version update introduces a breaking API change that will cause widespread compilation failures.

Why it matters: In axum-test v19+, the TestServer::new() constructor signature changed from returning Result<TestServer, Error> to returning TestServer directly (panicking on failure). The codebase has approximately 50 usages following the pattern TestServer::new(...).expect("...") or .unwrap(). Since new() no longer returns a Result, calling .expect() or .unwrap() on the return value is a type error that will fail compilation.

Example from line 872 of dwctl/src/test/mod.rs:

let server = axum_test::TestServer::new(router).expect("Failed to create test server");
// After update: cannot call `.expect()` because `new()` returns `TestServer`, not `Result`

Suggested fix: Either:

  1. Stay on v18.x (minimal change): Change to axum-test = { version = "18.7.0" } to get latest v18 without breaking changes

  2. Embrace v20 (requires refactoring): Update all ~50 call sites to use one of:

    • TestServer::try_new(...).expect("...") (explicit error handling)
    • TestServer::new(...) without .expect() (rely on panic-on-failure behavior)

See docs.rs comparison: v18.4.1 vs v20.0.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This major version upgrade (v18 β†’ v20) introduces breaking API changes that will cause compilation failures.

Why it matters: In axum-test v19.0.0, TestServer::new() was changed to return TestServer directly instead of Result<TestServer, _>. It now panics on failure rather than returning an error. The codebase has 56 occurrences of TestServer::new(...).unwrap() or TestServer::new(...).expect("...") patterns that will fail to compile because you cannot call .unwrap() or .expect() on a non-Result type.

For example, in dwctl/src/test/mod.rs:872:

let server = axum_test::TestServer::new(router).expect("Failed to create test server");

This will produce a compilation error like: "no method named expect found for struct TestServer"

Suggested fix:

  1. Remove all .unwrap() and .expect() calls from TestServer::new() invocations across the codebase (56 locations total)
  2. Change pattern from:
    let server = TestServer::new(app).unwrap();
    to:
    let server = TestServer::new(app);
  3. If you want to preserve custom error messages on failure, note that panic messages will now come from the library itself

Alternatively, if you want to handle errors explicitly, you could use try_new() (added in v19) where appropriate, though this is typically unnecessary for test code.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: Consider pinning to "20.0.0" explicitly or using ">=20.0.0, <21" range to avoid unexpected breakage when v20.1+ releases with potentially higher MSRV requirements.

Why it matters: According to the axum-test README, v20.1.0 requires Rust 1.88+. If your CI runs on an older Rust version, allowing automatic patch/minor upgrades could break builds unexpectedly. The current pinned version "20.0.0" is good, but consider documenting the MSRV requirement.

Suggested fix: Either keep as-is (pinned), or add a comment noting the MSRV:

# axum-test v20+ requires Rust 1.88+
axum-test = { version = "20.0.0" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This version upgrade introduces a breaking API change that will cause compilation failures.

Why it matters: In axum-test v19+, TestServer::new() no longer returns Result<TestServer, Error> β€” it returns TestServer directly and panics on failure. The codebase has 56 usages of .expect() or .unwrap() on this method's return value, e.g.:

let server = axum_test::TestServer::new(router).expect("Failed to create test server");

This will fail to compile because TestServer does not have .expect() or .unwrap() methods.

Suggested fix: Either:

  1. Remove .expect()/.unwrap() calls: let server = TestServer::new(router);
  2. Or switch to try_new(): let server = TestServer::try_new(router).expect("Failed to create test server");

All 56 occurrences across the test suite must be updated before this dependency bump can compile.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This major version upgrade introduces a breaking API change that will cause compilation failures.

Why it matters: In axum-test v19.0.0+, TestServer::new() was changed to no longer return a Result<TestServer, E> β€” it now returns TestServer directly and panics on failure. The codebase has ~56 instances of .expect("Failed to create test server") or .unwrap() calls on TestServer::new() results. These will fail to compile because you cannot call .expect() on a non-Result type.

From the v19.0.0 release notes:

new() constructor no longer returns a Result, and instead panics on failure. Added try_new() to allow people to continue to catch the Result if needed.

Suggested fix: Either (a) remove all .expect() / .unwrap() calls since the function now panics directly, or (b) replace TestServer::new() with TestServer::try_new() where error handling is desired. Example:

// Before (v18.x):
let server = TestServer::new(app).expect("Failed to create test server");

// After (v19+/v20.x) - option A (simplest):
let server = TestServer::new(app);

// After (v19+/v20.x) - option B (if you need Result handling):
let server = TestServer::try_new(app).expect("Failed to create test server");

Affected files include:

  • dwctl/src/test/mod.rs (lines 873, 1440, 1501)
  • dwctl/src/lib.rs (line 3162)
  • dwctl/src/error_enrichment.rs (lines 440, 570, 610, 639, 723, 877)
  • dwctl/src/api/handlers/payments.rs (~20 instances)
  • dwctl/src/api/handlers/auth.rs (~25 instances)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This version bump introduces a breaking API change that will cause 47 test call sites to fail compilation.

Why it matters: In axum-test v19.0.0+, TestServer::new() no longer returns a Result β€” it returns TestServer directly and panics on failure. All existing calls using .unwrap() or .expect() will produce compile errors like "no method named unwrap found for struct TestServer".

Suggested fix: Either:

  1. Remove .unwrap() / .expect() from all TestServer::new() call sites (simplest β€” new behavior just panics on failure, which is fine for tests)
  2. Or use try_new() instead if you want explicit error handling in specific tests

Affected files: auth.rs (30), payments.rs (15), error_enrichment.rs (6), lib.rs (1), test/mod.rs (3)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: Consider updating to axum-test = "20.1.0" instead of 20.0.0.

Why it matters: According to the axum-test compatibility matrix, version 20.1.0 is the recommended match for axum 0.8.9+, while 20.0.0 targets axum 0.8.8. This project uses axum 0.8.9 (per Cargo.lock line 861).

Suggested fix: Change the version to 20.1.0 for optimal compatibility:

axum-test = { version = "20.1.0" }

Note: Version 20.0.0 should still work due to axum's backward compatibility, but 20.1.0 is the officially recommended pairing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This version bump introduces a breaking API change that will cause 66 compilation errors in this repository.

Why it matters: In axum-test v19.0.0+, TestServer::new() no longer returns a Result β€” it panics on failure directly. This codebase has widespread usage of .unwrap() and .expect() on the return value:

  • 56 occurrences of TestServer::new(...).unwrap()
  • 10 occurrences of TestServer::new(...).expect(...)

These will fail to compile because .unwrap() and .expect() methods don't exist on TestServer (only on Result).

Affected files include:

  • src/api/handlers/auth.rs (31 occurrences)
  • src/api/handlers/payments.rs (15 occurrences)
  • src/test/mod.rs (3 occurrences)
  • src/error_enrichment.rs (6 occurrences)
  • src/lib.rs (1 occurrence)

Suggested fix: Remove the .unwrap() and .expect() calls since new() now handles errors by panicking internally. For example:

// Before:
let server = TestServer::new(app).unwrap();

// After:
let server = TestServer::new(app);

Alternatively, use try_new() if you want explicit error handling at specific call sites.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: This update introduces breaking API changes that will cause compilation failures.

Why it matters: In axum-test v19.0.0, TestServer::new() was changed to return TestServer directly instead of Result<TestServer, Error>. It now panics on failure rather than returning an error. The codebase currently has 10 usages of TestServer::new(...).expect("...") which will fail to compile because .expect() is being called on a TestServer instance (not a Result).

From the v19.0.0 release notes:

new() constructor no longer returns a Result, and instead panics on failure.
Added try_new() to allow people to continue to catch the Result if needed.

Suggested fix: Either:

  1. Remove .expect("Failed to create test server") from all 10 call sites (simplest, since the method now panics on failure anyway)
  2. Replace TestServer::new() with TestServer::try_new() if explicit error handling is desired

Affected files:

  • dwctl/src/test/mod.rs (lines 873, 1440, 1501)
  • dwctl/src/lib.rs (line 3162)
  • dwctl/src/error_enrichment.rs (lines 440, 570, 610, 639, 723, 877)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking: Consider pinning to a specific minor version for stability.

Why it matters: Using "20.0.0" allows Cargo to upgrade to any v20.x version (e.g., 20.1.0, 20.2.0) which could introduce subtle behavioral changes. While semver should prevent breaking changes within a major version, test library behavior changes can still affect test outcomes.

Suggested fix: Use version = "20.0" to stay on the v20.0.x patch line, or keep as-is if you want to automatically receive minor updates (which is often acceptable for dev dependencies).

sqlx = { version = "0.8", features = [
"postgres",
"runtime-tokio-rustls",
Expand Down
Loading