Skip to content

Add missing GPA deinit assertions to binary payload tests - #56

Merged
christianhelle merged 2 commits into
mainfrom
copilot/fix-comments-review-thread
Jul 3, 2026
Merged

Add missing GPA deinit assertions to binary payload tests#56
christianhelle merged 2 commits into
mainfrom
copilot/fix-comments-review-thread

Conversation

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Nine test blocks in src/tests/binary_payload_tests.zig called test_utils.createTestAllocator() but omitted the required defer std.debug.assert(gpa.deinit() == .ok), silently disabling memory leak detection for those tests.

Changes

  • src/tests/binary_payload_tests.zig: Added the missing defer std.debug.assert(gpa.deinit() == .ok) immediately after const allocator = gpa.allocator() in all nine affected tests, making them consistent with the full pattern used everywhere else in the test suite:
var gpa = test_utils.createTestAllocator();
const allocator = gpa.allocator();
defer std.debug.assert(gpa.deinit() == .ok);  // was missing in 9 tests

Already addressed (verified in current code)

The other review comments were resolved by the prior commit and are confirmed present:

  • classifyBody strips media-type parameters via media_type.baseMediaType() before comparison
  • convertRequestBody (v3.0/3.1/3.2 converters) uses mime.selectBestJsonKey() for normalized, deterministic JSON-first selection
  • selectConsumesMedia (Swagger v2) uses mime.isJson()/mime.isJsonSuffix() rather than verbatim string comparison
  • generateFunctionBodyDirect forces application/json content-type when falling back from form encoding
  • Generator emits requestRawWithContentType (not requestRaw) for non-application/json JSON media types (e.g. application/vnd.api+json)
  • "type" is present in the isReservedIdent list in both model_generator.zig and api_generator.zig

…load tests

Nine test blocks in binary_payload_tests.zig used createTestAllocator()
but omitted the required `defer std.debug.assert(gpa.deinit() == .ok)`
assertion, disabling memory leak detection for those tests.

Add the missing defer immediately after `const allocator = gpa.allocator()`
in each of the nine affected tests to restore full leak detection coverage.
Copilot AI changed the title [WIP] Fix code based on review comments fix(tests): add missing GPA deinit assertions to binary payload tests Jun 21, 2026
Copilot AI requested a review from christianhelle June 21, 2026 21:58
@christianhelle christianhelle changed the title fix(tests): add missing GPA deinit assertions to binary payload tests Add missing GPA deinit assertions to binary payload tests Jun 22, 2026
Base automatically changed from support-binary-payloads-opus to main June 22, 2026 10:53
@christianhelle
christianhelle marked this pull request as ready for review June 22, 2026 10:54
@christianhelle
christianhelle merged commit eac43f7 into main Jul 3, 2026
10 checks passed
@christianhelle
christianhelle deleted the copilot/fix-comments-review-thread branch July 3, 2026 18:10
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.

2 participants