Skip to content

Add tests for ID scalar coercion with Guid and int64 values#572

Draft
Copilot wants to merge 5 commits into
devfrom
copilot/fix-object-must-implement-iconvertible-error
Draft

Add tests for ID scalar coercion with Guid and int64 values#572
Copilot wants to merge 5 commits into
devfrom
copilot/fix-object-must-implement-iconvertible-error

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

Using IDType (GraphQL built-in ScalarDefinition<string>) with a System.Guid .NET field previously threw "Object must implement IConvertible" because the old coerceIDValue<t> used Convert.ChangeType, which requires IConvertible.

The fix was already present in coerceIdValue via the | _ -> Some(string x) fallback (calling ToString() on any value). This PR adds test coverage to lock in that behaviour.

Changes

  • ExecutionTests.fs: Two end-to-end execution tests exercising coerceIdValue with non-string values through the AutoField path:
    • Guid-backed ID field — the exact scenario from the original issue
    • int64-backed ID field — exercises the same | _ -> Some(string x) fallback branch with a different non-IConvertible type
type PersonWithGuidId  = { Id: Guid;  Name: string }
type PersonWithInt64Id = { Id: int64; Name: string }

// Both use AutoField so coerceIdValue receives the boxed native value (Guid / int64),
// hitting the | _ -> Some(string x) branch rather than the :? string fast path.
let PersonType =
    Define.Object<PersonWithGuidId>(
        "Person",
        [ Define.AutoField("id", IDType)   // was crashing before fix
          Define.AutoField("name", StringType) ])

Note: explicit Define.Field(..., IDType, resolver) resolvers are constrained by F# type inference to return string (because IDType : ScalarDefinition<string>), so Guid/int64 coercion can only be exercised end-to-end through AutoField.

  • CoercionTests.fs: Direct CoerceOutput unit tests verifying IDType serializes string, int, int64, and Guid values to their expected string representations.

Copilot AI linked an issue May 17, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix 'Object must implement IConvertible' error for ID GraphQL type Add tests for ID scalar coercion with Guid values May 17, 2026
Copilot AI requested a review from xperiandri May 17, 2026 14:55
@xperiandri xperiandri force-pushed the copilot/fix-object-must-implement-iconvertible-error branch from bb9dd5b to 0171893 Compare May 17, 2026 15:13
Copilot AI and others added 4 commits May 17, 2026 19:49
…ory` and remove external server orchestration from build (#564)

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Andrii Chebukin <XperiAndri@Outlook.com>
…compile time (#569)

Co-authored-by: xperiandri <2365592+xperiandri@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Andrii Chebukin <XperiAndri@Outlook.com>
@xperiandri xperiandri force-pushed the copilot/fix-object-must-implement-iconvertible-error branch from 0171893 to 99ffff5 Compare May 17, 2026 18:05
@xperiandri xperiandri requested a review from Copilot May 17, 2026 18:05
Copy link
Copy Markdown

Copilot AI left a comment

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 adds regression coverage for ID scalar output coercion when .NET values include Guid, especially around GraphQL IDType serialization.

Changes:

  • Adds direct IDType.CoerceOutput tests for string, int, int64, and Guid values.
  • Adds execution tests for a Person object with a Guid-backed ID field through AutoField and explicit Field definitions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/CoercionTests.fs Adds scalar output coercion coverage for IDType.
tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs Adds end-to-end execution tests for Guid-backed ID fields.

let PersonType =
Define.Object<PersonWithGuidId>(
"Person",
[ Define.Field("id", IDType, fun _ (p : PersonWithGuidId) -> string p.Id)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot is it true?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the reviewer was correct. Because IDType : ScalarDefinition<string>, the F# type system constrains any explicit Define.Field(..., IDType, ...) resolver to return string — so coerceIdValue only ever saw a boxed string, never a Guid.

Fixed in 3a79315: replaced that test with Execution handles ID scalar: serializes int64 field via AutoField, which uses AutoField on a PersonWithInt64Id.Id : int64 field. The auto-generated resolver returns a boxed int64, exercising the | _ -> Some(string x) fallback branch in coerceIdValue with a different non-IConvertible-reliant type.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copilot AI changed the title Add tests for ID scalar coercion with Guid values Add tests for ID scalar coercion with Guid and int64 values May 17, 2026
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.

"Object must implement IConvertible" when using built-in ID

3 participants