Skip to content

Add integer enum tests to existing composed enum tests#24348

Draft
wing328 wants to merge 2 commits into
masterfrom
followup-23869
Draft

Add integer enum tests to existing composed enum tests#24348
wing328 wants to merge 2 commits into
masterfrom
followup-23869

Conversation

@wing328

@wing328 wing328 commented Jul 19, 2026

Copy link
Copy Markdown
Member

a follow up pr to #23869

PR checklist

  • Read the contribution guidelines.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (For Windows users, please run the script in WSL)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Summary by cubic

Add integer enum coverage to the C# generichost composed-enum tests. Adds TypeIntegerWithOneOf (int32 oneOf/const) and folds the old annotated enum sample into ComposedEnum.

  • Refactors
    • Added TypeIntegerWithOneOf to the 3.0 composed-enum spec and generated sample; registered TypeIntegerWithOneOfJsonConverter.
    • Removed bin/configs/csharp-generichost-latest-annotatedEnum.yaml and the AnnotatedEnum sample; tests now live under ComposedEnum.
    • Updated docs, OpenAPI, and tests for the new integer enum.
    • Updated .github/workflows/samples-dotnet10.yaml to remove the deleted AnnotatedEnum sample from the CI matrix.

Written for commit d465c96. Summary will update on new commits.

Review in cubic

@wing328
wing328 marked this pull request as ready for review July 19, 2026 16:25

@cubic-dev-ai cubic-dev-ai Bot 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.

1 issue found across 90 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="samples/client/petstore/csharp/generichost/latest/ComposedEnum/src/Org.OpenAPITools/Model/TypeIntegerWithOneOf.cs">

<violation number="1" location="samples/client/petstore/csharp/generichost/latest/ComposedEnum/src/Org.OpenAPITools/Model/TypeIntegerWithOneOf.cs:31">
P1: The generated `TypeIntegerWithOneOf` model class cannot actually represent the integer enum values (1, 2, 4) defined in the spec. Unlike the existing string enums (`AreaCode`, `StateTerritoryCode`, `MarineAreaCode`) in the same project — which generate a proper C# `enum` type with a `ValueConverter` and correct serialize/deserialize behavior — this model is an empty class with no value property.

**What goes wrong:**
- The `Read` method requires `StartObject`/`StartArray` tokens, so a plain integer like `1` (the correct JSON wire format for `type: integer, format: int32`) throws `JsonException`.
- Even when reading an object, the `Read` method discards parsed data and returns `new TypeIntegerWithOneOf()` — the actual value is never captured. See the switch statement with only `default: break;`.
- `WriteProperties` is empty, so serialization always produces `{}` — all value information is lost on write.
- The class has no property or field to hold the integer value at all.

This means the type cannot roundtrip any value through JSON serialization, making it functionally incomplete as an integer enum representation. The existing string-based enums in this project (e.g., `AreaCode` with `AreaCodeValueConverter`, proper `JsonConverter<AreaCode>`) provide the correct pattern. Consider whether the generator template for `oneOf`/`const` on integer types needs to produce a model class with a value property and corresponding converter logic, similar to how the string enums work.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

/// TypeIntegerWithOneOf
/// </summary>
public partial class Parent : IValidatableObject
public partial class TypeIntegerWithOneOf : IValidatableObject

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.

P1: The generated TypeIntegerWithOneOf model class cannot actually represent the integer enum values (1, 2, 4) defined in the spec. Unlike the existing string enums (AreaCode, StateTerritoryCode, MarineAreaCode) in the same project — which generate a proper C# enum type with a ValueConverter and correct serialize/deserialize behavior — this model is an empty class with no value property.

What goes wrong:

  • The Read method requires StartObject/StartArray tokens, so a plain integer like 1 (the correct JSON wire format for type: integer, format: int32) throws JsonException.
  • Even when reading an object, the Read method discards parsed data and returns new TypeIntegerWithOneOf() — the actual value is never captured. See the switch statement with only default: break;.
  • WriteProperties is empty, so serialization always produces {} — all value information is lost on write.
  • The class has no property or field to hold the integer value at all.

This means the type cannot roundtrip any value through JSON serialization, making it functionally incomplete as an integer enum representation. The existing string-based enums in this project (e.g., AreaCode with AreaCodeValueConverter, proper JsonConverter<AreaCode>) provide the correct pattern. Consider whether the generator template for oneOf/const on integer types needs to produce a model class with a value property and corresponding converter logic, similar to how the string enums work.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/petstore/csharp/generichost/latest/ComposedEnum/src/Org.OpenAPITools/Model/TypeIntegerWithOneOf.cs, line 31:

<comment>The generated `TypeIntegerWithOneOf` model class cannot actually represent the integer enum values (1, 2, 4) defined in the spec. Unlike the existing string enums (`AreaCode`, `StateTerritoryCode`, `MarineAreaCode`) in the same project — which generate a proper C# `enum` type with a `ValueConverter` and correct serialize/deserialize behavior — this model is an empty class with no value property.

**What goes wrong:**
- The `Read` method requires `StartObject`/`StartArray` tokens, so a plain integer like `1` (the correct JSON wire format for `type: integer, format: int32`) throws `JsonException`.
- Even when reading an object, the `Read` method discards parsed data and returns `new TypeIntegerWithOneOf()` — the actual value is never captured. See the switch statement with only `default: break;`.
- `WriteProperties` is empty, so serialization always produces `{}` — all value information is lost on write.
- The class has no property or field to hold the integer value at all.

This means the type cannot roundtrip any value through JSON serialization, making it functionally incomplete as an integer enum representation. The existing string-based enums in this project (e.g., `AreaCode` with `AreaCodeValueConverter`, proper `JsonConverter<AreaCode>`) provide the correct pattern. Consider whether the generator template for `oneOf`/`const` on integer types needs to produce a model class with a value property and corresponding converter logic, similar to how the string enums work.</comment>

<file context>
@@ -26,45 +26,29 @@
+    /// TypeIntegerWithOneOf
     /// </summary>
-    public partial class Parent : IValidatableObject
+    public partial class TypeIntegerWithOneOf : IValidatableObject
     {
         /// <summary>
</file context>

@wing328
wing328 marked this pull request as draft July 19, 2026 16:43
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.

1 participant