Skip to content

Refactor: consolidate test helpers, parameterize tests, and fix WASM OOM bug#137

Merged
nao1215 merged 7 commits intomainfrom
refactor/code-review-fixes
Mar 13, 2026
Merged

Refactor: consolidate test helpers, parameterize tests, and fix WASM OOM bug#137
nao1215 merged 7 commits intomainfrom
refactor/code-review-fixes

Conversation

@nao1215
Copy link
Owner

@nao1215 nao1215 commented Mar 13, 2026

Summary

  • Consolidate duplicated test helpers into tests/common/mod.rs: tiny_png(), send_signed_get(), status_code(), eliminating ~200 lines of duplication across S3/GCS/Azure integration tests
  • Parameterize HEAD request tests using rstest, replacing 4 nearly identical test functions with a single parameterized test
  • Remove unnecessary .clone() in auth.rs parse_query_params by using contains_key before insert
  • Fix misplaced doc comment for tiny_png / large_png_bytes in tests/common/mod.rs
  • Fix WASM OOM bug: add #[serde(skip_serializing)] to WasmTransformResponse.bytes to prevent serializing megabytes of image data as a JSON number array, which caused browser hangs when drag-and-dropping images on GitHub Pages

Test plan

  • cargo fmt --all -- --check passes
  • cargo clippy --all-targets --all-features -- -D warnings passes
  • cargo test passes
  • Verified WASM build is unaffected (JS accesses bytes via wasm-bindgen getter, not JSON)

Summary by CodeRabbit

  • Bug Fixes

    • Improved query parameter validation to explicitly reject duplicate parameters with HTTP 400 Bad Request responses.
  • Improvements

    • Optimized WASM response serialization by excluding binary bytes from JSON payloads, reducing message size.
  • Tests

    • Refactored test utilities into a shared module to reduce duplication across integration tests.

@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR adds test infrastructure improvements by introducing rstest-based parameterized testing, consolidating test utilities into a shared common module, refactoring integration tests to reduce duplication, fixing duplicate query parameter handling in auth parsing, and excluding large binary data from JSON serialization in WASM responses.

Changes

Cohort / File(s) Summary
Dependencies & Test Framework
Cargo.toml, tests/server_head.rs
Added rstest dev-dependency and refactored HEAD request tests to use rstest parameterization, replacing four individual test functions with a single parameterized test covering all cases.
Test Utilities Consolidation
tests/common/mod.rs, tests/azure_integration.rs, tests/gcs_integration.rs, tests/s3_integration.rs
Added shared test utilities (tiny_png, send_signed_get, status_code) to common module and refactored three integration test files to replace in-file helper implementations with calls to centralized common module functions, reducing duplication.
Server Logic & Serialization
src/adapters/server/auth.rs, src/adapters/wasm.rs
Fixed duplicate query parameter detection in parse_query_params by checking for existing keys before insertion, and added #[serde(skip_serializing)] to exclude large binary bytes from WASM transformation response JSON payloads.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 In common burrows, helpers now reside,
Query params checked with proper pride,
With rstest's hop through parameterized tests,
And bytes that skip the JSON's manifest,
Our codebase hops its very best! 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the three main changes: test helper consolidation, test parameterization, and the WASM OOM bug fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/code-review-fixes
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nao1215 nao1215 merged commit 3a1ff71 into main Mar 13, 2026
16 of 17 checks passed
@nao1215 nao1215 deleted the refactor/code-review-fixes branch March 13, 2026 11:46
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.

1 participant