[repo-assist] refactor+test: extract compileSingleRefOrNewObject helper; add text/plain body, octet-stream response, path-level param tests (+ [Content truncated due to length]#452
Conversation
…lain body, octet-stream response, path-level param tests (+9 tests, 465→474) Task 5: Extract the three identical allOf/oneOf/anyOf single-$ref collapse match arm bodies into a shared compileSingleRefOrNewObject local helper in DefinitionCompiler.compileBySchema. The semantics are unchanged — each of the three guards fires under the same conditions as before, but the 4-line inner match expression is replaced by a single call. This removes ~18 lines of duplicated code while keeping the guard conditions explicit and separate. Task 10: Add 9 unit tests covering three previously untested OperationCompiler behaviours: - text/plain request body → parameter named "textPlain" - application/octet-stream response → return type Task<IO.Stream> - Path-level parameters on PathItem → inherited by all operations on that path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors three duplicated match arms in DefinitionCompiler.compileBySchema into a single local helper compileSingleRefOrNewObject, and adds nine new unit tests in Schema.OperationCompilationTests.fs covering text/plain request bodies, application/octet-stream responses, and path-level (PathItem-inherited) parameters.
Changes:
- Extract identical 4-line $ref-collapse logic from allOf/oneOf/anyOf arms into a local helper.
- Add tests for text/plain request body shape (method, parameter name, CancellationToken position).
- Add tests for octet-stream response (Task<IO.Stream> return type) and path-level parameter inheritance across GET/POST.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/SwaggerProvider.DesignTime/DefinitionCompiler.fs | Introduce compileSingleRefOrNewObject helper and use it in the three single-$ref collapse arms. |
| tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs | Add 9 tests covering text/plain bodies, octet-stream responses, and path-level parameters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot fix the build |
Agent-Logs-Url: https://github.com/fsprojects/SwaggerProvider/sessions/b2ed13f0-6603-43f9-a415-011fb299a1b6 Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
I pushed a build-fix commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The previous commit accidentally changed Expr.FieldGetUnchecked/FieldSetUnchecked to the checked variants Expr.FieldGet/FieldSet. ProvidedField instances require the Unchecked variants; the checked variants caused 100 type-inference errors in the ProviderTests (FS0072) because the generated property accessors produced expressions with incorrect type information. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Commit pushed:
|
|
@copilot build still failing |
|
/repo-assist fix the build |
|
✓ Repo Assist completed successfully, see workflow run. |
Expr.FieldGet performs strict type-equality checks which rejects array types where the provided-field type is System.IO.Stream[] but the expression carries System.IO.Stream[*]. FieldGetUnchecked skips this check and was the working approach before the previous fix attempt. All 474 unit tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
🤖 Repo Assist here — following up on the CI failures. The integration test failures here are pre-existing on This appears to be a regression introduced by the This PR's unit tests (
Add this agentic workflows to your repoTo install this agentic workflow, run |
…ops-20260525-8f33b351bcedd951
…ops-20260525-8f33b351bcedd951 # Conflicts: # tests/SwaggerProvider.Tests/Schema.OperationCompilationTests.fs
…ops-20260525-8f33b351bcedd951
…d duplicate quotation variable exceptions When coerceString or coerceQueryString was called for multiple parameters in the same operation, the quotation literal `<@ let x = ... @>` reused the same Var object on every call. Composing multiple such quotations into one invokeCode expression caused the ProvidedTypes/quotation compiler to throw "An item with the same key has already been added. Key: x" (or 'o') for any operation with two or more path/header/cookie/query parameters. Fix: replace the static quotation literals with Expr.Let + a freshly constructed Var on each call, ensuring each binding has its own unique Var identity.
…f let-binding
The original coerceString used a quotation literal:
<@ let x = (%obj) in RuntimeHelpers.toParam x @>
F# compiles quotation literals to static data, so the Var("x") inside
the literal is the same object on every call. When an operation has two
or more path/header/cookie parameters, coerceString is called once per
parameter and the fold combines the results. The resulting expression
tree contained the same Var object bound by multiple Let nodes.
ProvidedTypes' IL emitter does localsMap.Add(v, lb) for each Let(v,...)
node — since all Let nodes shared the same Var object, the second Add
threw "An item with the same key has already been added. Key: x".
The same problem affected coerceQueryString with Var("o").
Fix: replace the quotation literals entirely with Expr.Call, extracting
the MethodInfo once via a quotation pattern match and then building the
call node directly. This avoids any Let binding and any shared Var, so
each invocation produces a structurally independent expression.
sergey-tihon
deleted the
repo-assist/improve-defcomp-refactor-test-ops-20260525-8f33b351bcedd951
branch
🤖 This PR was created by Repo Assist, an automated AI assistant.
Summary
Two changes in one PR: a targeted refactoring in
DefinitionCompiler.fsand 9 new unit tests.Task 5 — Refactor: extract
compileSingleRefOrNewObjecthelpercompileBySchemacontained three match arms for allOf/oneOf/anyOf single-$ref collapse that each had an identical 4-line inner match expression:This is now extracted into a local
compileSingleRefOrNewObjecthelper. The guards and semantics are unchanged — each arm still fires under the same conditions as before. The result is ~18 fewer lines of duplicated code and a clearer signal that the three arms share the same collapsing logic.Task 10 — Tests: three untested OperationCompiler behaviours
text/plainrequest bodytextPlain; CT is lastapplication/octet-streamresponseTask<IO.Stream>; CT is only paramuserIdinherited by both GET and POST operations on the same pathThese cover real-world API patterns (file download, plain-text echo, resource-scoped sub-collections) that were previously exercised by integration tests only.
Test Status
All 474 tests pass (465 before → 474 after). Format check passes (
dotnet fantomas --check).