Skip to content

Merge master and fix CI: add RuntimeHelpers unit tests#305

Merged
sergey-tihon merged 4 commits into
repo-assist/improve-runtime-helpers-tests-2026-03-4685f59d81f67329from
copilot/sub-pr-295
Mar 10, 2026
Merged

Merge master and fix CI: add RuntimeHelpers unit tests#305
sergey-tihon merged 4 commits into
repo-assist/improve-runtime-helpers-tests-2026-03-4685f59d81f67329from
copilot/sub-pr-295

Conversation

Copilot AI commented Mar 10, 2026

Copy link
Copy Markdown
Contributor
  • Merge master branch (fixes global.json SDK version and adds unwrapFSharpOption helper)
  • Verify all 177 tests pass (0 failures, 2 pre-existing skips)
  • Fix "Build and Test" workflow: removed branches: [master] filter from pull_request trigger so the workflow runs for all PRs, including stacked PRs targeting non-master branches

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

dsyme and others added 2 commits March 10, 2026 21:58
RuntimeHelpers.getPropertyValues now strips the FSharpOption<T>
wrapper before returning property values. Previously, a property
set to Some(true) was serialised as the string "Some(true)" in
multipart/form-data and application/x-www-form-urlencoded bodies
instead of the bare "True". None values were already handled (null).

Also relax global.json SDK version constraint from 10.0.103 to
10.0.100 with rollForward:latestPatch so the project builds with
any 10.0.1xx SDK available locally or in CI.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI changed the title [WIP] Add unit tests for RuntimeHelpers module Merge master and fix CI: add RuntimeHelpers unit tests Mar 10, 2026
@sergey-tihon

Copy link
Copy Markdown
Member

@copilot why this PR does not run "Build and Test" workflow? fix it

… targeting master

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

Copilot AI commented Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

@copilot why this PR does not run "Build and Test" workflow? fix it

Fixed in commit 26f4e90. The workflow had branches: [master] on its pull_request trigger, which prevented it from running on PRs targeting non-master branches (like this stacked PR). Removed that restriction so "Build and Test" now runs for all pull requests.

@sergey-tihon sergey-tihon marked this pull request as ready for review March 10, 2026 21:22
Copilot AI review requested due to automatic review settings March 10, 2026 21:22
@sergey-tihon sergey-tihon merged commit 3c8a90e into repo-assist/improve-runtime-helpers-tests-2026-03-4685f59d81f67329 Mar 10, 2026
8 checks passed
@sergey-tihon sergey-tihon deleted the copilot/sub-pr-295 branch March 10, 2026 21:22

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

Updates runtime form-serialization behavior and CI configuration after merging master, aiming to fix CI stability and prevent F# option values from being serialized as Some(value) in form payloads.

Changes:

  • Unwrap F# option<_> property values during form/form-url-encoded serialization.
  • Adjust global.json SDK pin/roll-forward behavior.
  • Update GitHub Actions workflow trigger so PR builds run for non-master target branches.

Reviewed changes

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

File Description
src/SwaggerProvider.Runtime/RuntimeHelpers.fs Unwraps F# option values before emitting form key/value pairs.
global.json Pins .NET SDK version and changes rollForward policy.
.github/workflows/dotnetcore.yml Removes pull_request branch filter so CI runs for all PR targets.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +148 to +162
// Unwraps F# option values: returns the inner value for Some, null for None.
// This prevents `Some(value)` from being sent as-is in form data.
let private unwrapFSharpOption(value: obj) : obj =
if isNull value then
null
else
let ty = value.GetType()

if
ty.IsGenericType
&& ty.GetGenericTypeDefinition() = typedefof<option<_>>
then
ty.GetProperty("Value").GetValue(value)
else
value

Copilot AI Mar 10, 2026

Copy link

Choose a reason for hiding this comment

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

The comment says this returns “inner value for Some, null for None”, but this implementation also returns null for Some null (because it unwraps to the inner null). That means Some null will be treated the same as None downstream (e.g., dropped by Option.ofObj). Please clarify the comment to reflect the actual behavior, or adjust the unwrapping logic if preserving Some null is important for form serialization.

Copilot uses AI. Check for mistakes.
Comment thread global.json
"version": "10.0.103",
"rollForward": "minor"
"version": "10.0.100",
"rollForward": "latestPatch"

Copilot AI Mar 10, 2026

Copy link

Choose a reason for hiding this comment

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

global.json now pins the SDK to feature band 10.0.100 with rollForward: latestPatch. This requires a 10.0.1xx SDK to be installed; it will not roll forward to a newer feature band (e.g., 10.0.2xx). The workflow currently installs 10.0.x, which may resolve to a different feature band over time and cause dotnet to fail with “SDK not found” when global.json can’t be satisfied. Consider aligning CI with global.json (e.g., install 10.0.100/10.0.1xx, or configure setup-dotnet to use the global.json), or relax rollForward back to a mode that allows feature-band roll-forward if that’s intended.

Suggested change
"rollForward": "latestPatch"
"rollForward": "latestFeature"

Copilot uses AI. Check for mistakes.
sergey-tihon added a commit that referenced this pull request Mar 10, 2026
* Add unit tests for RuntimeHelpers module

RuntimeHelpers.fs handles all parameter serialization and HTTP request
building for generated API clients, but previously had zero dedicated
unit tests. This PR adds 44 focused tests covering:

- toParam: DateTime/DateTimeOffset ISO 8601 formatting, null handling
- toQueryParams: all supported array/option/scalar types, DateTime
  ISO 8601 serialization, None filtering in optional arrays
- combineUrl: trailing/leading slash normalisation
- createHttpRequest: HTTP method, path, query params, null filtering
- fillHeaders: header addition, null-value skip, Content-Type silence
- toStringContent/toTextContent/toStreamContent: content-type headers,
  error on non-stream input

Closes no specific issue; improves baseline test coverage for the
runtime serialization layer (Task 9 - Testing Improvements).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* ci: trigger checks

* Add missing toQueryParams test cases: float32/double/byte arrays and Option array types (#304)

* Initial plan

* Add missing toQueryParams test cases: float32/double/byte arrays and Option array types

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Merge master and fix CI: add RuntimeHelpers unit tests (#305)

* Fix Option types in form data not being unwrapped (issue #214) (#298)

RuntimeHelpers.getPropertyValues now strips the FSharpOption<T>
wrapper before returning property values. Previously, a property
set to Some(true) was serialised as the string "Some(true)" in
multipart/form-data and application/x-www-form-urlencoded bodies
instead of the bare "True". None values were already handled (null).

Also relax global.json SDK version constraint from 10.0.103 to
10.0.100 with rollForward:latestPatch so the project builds with
any 10.0.1xx SDK available locally or in CI.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Initial plan

* Fix Build and Test workflow to run on all pull requests, not just PRs targeting master

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Remove unreachable null check from `toQueryParams` fallthrough case (#306)

* Initial plan

* Fix toQueryParams: remove unreachable null check from _ case

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Make null handling explicit in `toQueryParams` and fix misleading test comment (#307)

* Initial plan

* Add explicit null guard in toQueryParams and fix test comment

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Apply Fantomas formatting to RuntimeHelpers.fs

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
sergey-tihon added a commit that referenced this pull request Mar 10, 2026
* Fix Option types in form data not being unwrapped (issue #214) (#298)

RuntimeHelpers.getPropertyValues now strips the FSharpOption<T>
wrapper before returning property values. Previously, a property
set to Some(true) was serialised as the string "Some(true)" in
multipart/form-data and application/x-www-form-urlencoded bodies
instead of the bare "True". None values were already handled (null).

Also relax global.json SDK version constraint from 10.0.103 to
10.0.100 with rollForward:latestPatch so the project builds with
any 10.0.1xx SDK available locally or in CI.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add concurrency group to CI workflow to cancel redundant builds (#309)

When multiple commits are pushed to the same PR branch, GitHub Actions
queues a new build for each push. Without a concurrency group, the older
builds keep running even though they're now stale.

Adding cancel-in-progress: true cancels the in-progress build when a
newer commit arrives on the same ref, reducing CI minutes consumed and
giving faster feedback on the latest commit.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* [Repo Assist] Add unit tests for RuntimeHelpers module (#295)

* Add unit tests for RuntimeHelpers module

RuntimeHelpers.fs handles all parameter serialization and HTTP request
building for generated API clients, but previously had zero dedicated
unit tests. This PR adds 44 focused tests covering:

- toParam: DateTime/DateTimeOffset ISO 8601 formatting, null handling
- toQueryParams: all supported array/option/scalar types, DateTime
  ISO 8601 serialization, None filtering in optional arrays
- combineUrl: trailing/leading slash normalisation
- createHttpRequest: HTTP method, path, query params, null filtering
- fillHeaders: header addition, null-value skip, Content-Type silence
- toStringContent/toTextContent/toStreamContent: content-type headers,
  error on non-stream input

Closes no specific issue; improves baseline test coverage for the
runtime serialization layer (Task 9 - Testing Improvements).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* ci: trigger checks

* Add missing toQueryParams test cases: float32/double/byte arrays and Option array types (#304)

* Initial plan

* Add missing toQueryParams test cases: float32/double/byte arrays and Option array types

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Merge master and fix CI: add RuntimeHelpers unit tests (#305)

* Fix Option types in form data not being unwrapped (issue #214) (#298)

RuntimeHelpers.getPropertyValues now strips the FSharpOption<T>
wrapper before returning property values. Previously, a property
set to Some(true) was serialised as the string "Some(true)" in
multipart/form-data and application/x-www-form-urlencoded bodies
instead of the bare "True". None values were already handled (null).

Also relax global.json SDK version constraint from 10.0.103 to
10.0.100 with rollForward:latestPatch so the project builds with
any 10.0.1xx SDK available locally or in CI.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Initial plan

* Fix Build and Test workflow to run on all pull requests, not just PRs targeting master

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Remove unreachable null check from `toQueryParams` fallthrough case (#306)

* Initial plan

* Fix toQueryParams: remove unreachable null check from _ case

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Make null handling explicit in `toQueryParams` and fix misleading test comment (#307)

* Initial plan

* Add explicit null guard in toQueryParams and fix test comment

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Apply Fantomas formatting to RuntimeHelpers.fs

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Co-authored-by: Don Syme <dsyme@users.noreply.github.com>

* Initial plan

---------

Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Sergey Tihon <sergey.tihon@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
sergey-tihon added a commit that referenced this pull request Mar 10, 2026
…eference (#310)

* Fix $0 in path parameter values treated as regex back-reference (#141)

Regex.Replace uses $0, $1, $& etc. as back-references in the replacement
string. When a path parameter value contained a literal dollar sign (e.g.
"$0something"), Regex.Replace would substitute it with the matched text
instead of the literal value.

Fix: escape $ characters in the replacement string by replacing "$" with
"$$" (the .NET replacement-string escape for a literal dollar sign).
This applies to both the v2 SwaggerClientProvider and v3 OpenApiClientProvider.

Also adds:
- EchoPathController in the test server that returns its string path parameter
- A regression test in SpecialCasesControllers.Tests.fs that passes "$0something"
  through a path parameter and asserts the value is echoed back unchanged
- global.json rollForward updated from minor/10.0.103 to latestPatch/10.0.100
  so the project builds with any 10.0.1xx SDK available in CI

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix $ in path parameters treated as Regex.Replace back-references (#311)

* Fix Option types in form data not being unwrapped (issue #214) (#298)

RuntimeHelpers.getPropertyValues now strips the FSharpOption<T>
wrapper before returning property values. Previously, a property
set to Some(true) was serialised as the string "Some(true)" in
multipart/form-data and application/x-www-form-urlencoded bodies
instead of the bare "True". None values were already handled (null).

Also relax global.json SDK version constraint from 10.0.103 to
10.0.100 with rollForward:latestPatch so the project builds with
any 10.0.1xx SDK available locally or in CI.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add concurrency group to CI workflow to cancel redundant builds (#309)

When multiple commits are pushed to the same PR branch, GitHub Actions
queues a new build for each push. Without a concurrency group, the older
builds keep running even though they're now stale.

Adding cancel-in-progress: true cancels the in-progress build when a
newer commit arrives on the same ref, reducing CI minutes consumed and
giving faster feedback on the latest commit.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* [Repo Assist] Add unit tests for RuntimeHelpers module (#295)

* Add unit tests for RuntimeHelpers module

RuntimeHelpers.fs handles all parameter serialization and HTTP request
building for generated API clients, but previously had zero dedicated
unit tests. This PR adds 44 focused tests covering:

- toParam: DateTime/DateTimeOffset ISO 8601 formatting, null handling
- toQueryParams: all supported array/option/scalar types, DateTime
  ISO 8601 serialization, None filtering in optional arrays
- combineUrl: trailing/leading slash normalisation
- createHttpRequest: HTTP method, path, query params, null filtering
- fillHeaders: header addition, null-value skip, Content-Type silence
- toStringContent/toTextContent/toStreamContent: content-type headers,
  error on non-stream input

Closes no specific issue; improves baseline test coverage for the
runtime serialization layer (Task 9 - Testing Improvements).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* ci: trigger checks

* Add missing toQueryParams test cases: float32/double/byte arrays and Option array types (#304)

* Initial plan

* Add missing toQueryParams test cases: float32/double/byte arrays and Option array types

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Merge master and fix CI: add RuntimeHelpers unit tests (#305)

* Fix Option types in form data not being unwrapped (issue #214) (#298)

RuntimeHelpers.getPropertyValues now strips the FSharpOption<T>
wrapper before returning property values. Previously, a property
set to Some(true) was serialised as the string "Some(true)" in
multipart/form-data and application/x-www-form-urlencoded bodies
instead of the bare "True". None values were already handled (null).

Also relax global.json SDK version constraint from 10.0.103 to
10.0.100 with rollForward:latestPatch so the project builds with
any 10.0.1xx SDK available locally or in CI.

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Initial plan

* Fix Build and Test workflow to run on all pull requests, not just PRs targeting master

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Remove unreachable null check from `toQueryParams` fallthrough case (#306)

* Initial plan

* Fix toQueryParams: remove unreachable null check from _ case

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Make null handling explicit in `toQueryParams` and fix misleading test comment (#307)

* Initial plan

* Add explicit null guard in toQueryParams and fix test comment

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

* Apply Fantomas formatting to RuntimeHelpers.fs

Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
Co-authored-by: Don Syme <dsyme@users.noreply.github.com>

* Initial plan

---------

Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Sergey Tihon <sergey.tihon@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

4 participants