Skip to content

[repo-assist] refactor: deduplicate CallAsync error branch; add stream-array and HTTP-fallback tests#460

Merged
sergey-tihon merged 2 commits into
masterfrom
repo-assist/improve-callasyncs-dedup-tests-2025-06-ef2fa682642f9394
Jun 13, 2026
Merged

[repo-assist] refactor: deduplicate CallAsync error branch; add stream-array and HTTP-fallback tests#460
sergey-tihon merged 2 commits into
masterfrom
repo-assist/improve-callasyncs-dedup-tests-2025-06-ef2fa682642f9394

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

ProvidedApiClientBase.CallAsync had two nearly identical error branches — both called readBody() and then raise(OpenApiException(...)) with different description strings. This PR merges them into a single readBody() call followed by a single raise, with an if/elif/else expression selecting the description.

Changes

src/SwaggerProvider.Runtime/ProvidedApiClientBase.fs

  • Extracted readBody() and the errorIdx lookup before the description-selection branch
  • Removed the duplicated raise(OpenApiException(...)) from both branches, leaving one

tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs

  • Added EmptyReasonPhraseHandler — a test helper that creates an HttpResponseMessage with ReasonPhrase explicitly set to "", triggering the "HTTP {code}" fallback in CallAsync
  • Added test: toMultipartFormDataContent adds each stream in a Stream array as a separate part — covers the previously untested IO.Stream[] match arm in toMultipartFormDataContent
  • Added test: CallAsync uses HTTP status code in description when reason phrase is empty — verifies the "HTTP {code}" fallback path when ReasonPhrase is empty

Trade-offs

No behaviour change. The refactor makes the invariant explicit: the body is always read and the exception always raised; only the description varies.

Test Status

✅ Build: succeeded (0 errors, 12 warnings — all pre-existing)
✅ Tests: 512 passed, 0 failed, 1 skipped (pre-existing skip)
✅ Format: dotnet fantomas --check passes after formatting both changed files

Generated by 🌈 Repo Assist, see workflow run. Learn more.

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@e15e57b40918dbca11b350c55d02ab61934afa75

…TP-fallback tests

- Merge duplicate readBody()/raise(OpenApiException(...)) calls in
  CallAsync into a single error path; description selection is now
  the only branching point.
- Add test: toMultipartFormDataContent with IO.Stream[] input
- Add EmptyReasonPhraseHandler helper and test: CallAsync falls back
  to 'HTTP {code}' description when ReasonPhrase is empty

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sergey-tihon sergey-tihon marked this pull request as ready for review June 13, 2026 08:53
Copilot AI review requested due to automatic review settings June 13, 2026 08:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors ProvidedApiClientBase.CallAsync to remove duplicated error-handling code paths while keeping behavior the same, and it adds targeted runtime helper tests (including coverage for the stream-array multipart branch and an HTTP reason-phrase fallback).

Changes:

  • Deduplicated CallAsync error-response handling to read the body once and raise OpenApiException via a single shared branch.
  • Added a test handler to force an empty ReasonPhrase and a test asserting the "HTTP {code}" description fallback.
  • Added multipart form-data coverage for IO.Stream[] inputs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/SwaggerProvider.Runtime/ProvidedApiClientBase.fs Refactors error handling in CallAsync to eliminate duplicated body-read/exception-raise logic.
tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs Adds tests for empty reason-phrase fallback and Stream[] multipart parts (but currently needs a fix to separate two tests).
Comments suppressed due to low confidence (1)

tests/SwaggerProvider.Tests/RuntimeHelpersTests.fs:1314

  • The new stream-array test accidentally includes the body of the (now-missing) toMultipartFormDataContent skips values when toParam returns null test as a trailing task { ... } expression. This makes the new test do two unrelated things and removes coverage for the null-skipping behavior as a standalone test. Split these into two separate [<Fact>] tests.

        task {
            let nestedNone = box(Some(None: string option))

            use content =

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sergey-tihon sergey-tihon merged commit 20ffec2 into master Jun 13, 2026
3 checks passed
@sergey-tihon sergey-tihon deleted the repo-assist/improve-callasyncs-dedup-tests-2025-06-ef2fa682642f9394 branch June 13, 2026 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants