Skip to content

Add tests for not in python client - #25125

Merged
wing328 merged 1 commit into
masterfrom
python-test-not
Oct 5, 2026
Merged

wing328 merged 1 commit into
masterfrom
python-test-not

Conversation

@wing328

@wing328 wing328 commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

a follow up pr to #25070

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

Adds test schemas for the OpenAPI not keyword to the Python client test spec, generating new Known, Other, and TaggedUnion models with unit tests across all Python sample variants.

  • Known allows only the known kind value, Other rejects known and reserved via not, and TaggedUnion combines them in a oneOf.
  • Regenerates samples for aiohttp, httpx, httpx2, lazyImports, and pydantic-v1 variants so CI sample checks pass.

Written for commit 2533f20. Summary will update on new commits.

Review in cubic

@wing328 wing328 added this to the 7.26.0 milestone Oct 5, 2026
@wing328
wing328 marked this pull request as ready for review October 5, 2026 10:11

@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.

22 issues found across 118 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="samples/openapi3/client/petstore/python-httpx2/test/test_tagged_union.py">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx2/test/test_tagged_union.py:38">
P3: `make_instance` suggests `TaggedUnion(kind='')`, but `TaggedUnion` is a oneOf union model whose only settable fields are `actual_instance` / `oneof_schema_1_validator` / `oneof_schema_2_validator` (see `tagged_union.py`); it has no `kind` field. If the stub is uncommented, `kind` is silently ignored (pydantic `extra` defaults to ignore) and the helper returns an instance with `actual_instance=None`, so the example cannot produce a usable instance. Use `actual_instance` with a concrete schema, e.g. `TaggedUnion(actual_instance=Known(kind='known'))`. The root cause is the generator template merging `kind` from the oneOf member schemas into the union model stub, so the same defect appears in all five regenerated variants.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx/test/test_tagged_union.py">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx/test/test_tagged_union.py:46">
P3: These added test files contain no assertions: `make_instance` bodies and `testKnown`/`testOther`/`testTaggedUnion` are entirely commented-out stubs, so the tests pass without exercising anything, and the `not` behavior (Other rejecting `known`/`reserved`, Known accepting only `known`) is never verified. The PR description presents these as "unit tests" adding "test coverage" for `not`; as shipped they provide none. Add real assertions (e.g. `Other(kind='known')` raises, `Known(kind='known')` succeeds, `TaggedUnion.from_dict` picks the right schema), in the generator template or as hand-written sample tests, so a regression in `not` codegen is actually caught.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/docs/Other.md">

<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/docs/Other.md:19">
P3: The new doc's example code is invalid Python 3: `print Other.to_json()` is Python 2 print-statement syntax and will raise SyntaxError in the Python 3.7+ runtime this package requires, and it calls `to_json()` on the class instead of on `other_instance`. Fix the source template `modules/openapi-generator/src/main/resources/python-pydantic-v1/model_doc.mustache`, which still emits `print {{classname}}.to_json()`, to match the already-fixed python template (`print({{classname}}.to_json())`), then regenerate the samples.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-pydantic-v1/docs/Known.md">

<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1/docs/Known.md:19">
P3: The example block uses Python 2 print-statement syntax (`print Known.to_json()`), which is a `SyntaxError` in the Python 3.7+ this sample targets, so the docs example is not runnable. The root cause is the template `modules/openapi-generator/src/main/resources/python-pydantic-v1/model_doc.mustache` (line 23: `print {{classname}}.to_json()`); fix it there and regenerate so `Known.md`, `Other.md`, and `TaggedUnion.md` all get `print(...)`.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx-sync/test/test_other.py">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx-sync/test/test_other.py:46">
P2: The PR claims to add Python test coverage for the `not` keyword, but this added test is an empty stub: `make_instance` always returns `None` (its body is fully commented out, despite the `-> Other` annotation), and `testOther` makes no assertions. Nothing here or in the sibling `test_known.py` / `test_tagged_union.py` exercises the generated `not` validation, so the stated coverage is not actually added. Uncomment `make_instance` and add assertions that `Other` accepts a non-excluded value but raises `ValueError` for `known` and `reserved`.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx/test/test_known.py">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx/test/test_known.py:46">
P2: These added "tests" never execute: the `make_instance` body is a standalone string literal (the second `"""` block after `# uncomment below` is an inert string expression) and `testKnown` only contains commented-out calls, so both pass trivially. For a PR whose stated purpose is Python test coverage for OpenAPI's `not` keyword (rejecting `known`/`reserved` for `Other`), add executable assertions — e.g. `Other(kind='known')` raises ValueError, `Other(kind='foo')` is valid, and `TaggedUnion` accepts both — so a generator regression in the `not` handling would fail the suite.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/docs/TaggedUnion.md">

<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/docs/TaggedUnion.md:19">
P3: The example code line `print TaggedUnion.to_json()` is a Python 3 syntax error (`print` used as a statement), so the doc's example block will not run. Use `print(TaggedUnion.to_json())` as in the python (non-pydantic-v1) template. Root cause is `modules/openapi-generator/src/main/resources/python-pydantic-v1/model_doc.mustache` line 23, which emits `print {{classname}}.to_json()`; fix the template and regenerate rather than hand-editing this file.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx-sync/test/test_tagged_union.py">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx-sync/test/test_tagged_union.py:46">
P2: This "test" file, added to give `not`-keyword coverage, executes nothing: `make_instance`'s body is entirely commented out and `testTaggedUnion` is commented out, so neither the `Other` rejection of `kind: known/reserved` nor the `TaggedUnion` oneOf matching is ever exercised. `make_instance` also returns `None` despite the `-> TaggedUnion` annotation while it is commented out. The PR description claims Python test coverage for `not`, but these regenerated test files (in this and all other variants) contain no assertions. Since the samples are regenerated from templates, add real assertions in the generator's Python test templates or a generator unit test that constructs `Other(kind='known')` / validates `TaggedUnion` deserialization, rather than shipping inert stubs. Note the commented example is misleading too: `kind` is not a field of `TaggedUnion` — the oneOf instance is set via `actual_instance`/`Known`/`Other`.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx-sync/docs/Other.md">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx-sync/docs/Other.md:20">
P3: The example calls `print(Other.to_json())`, but `to_json` is an instance method (`def to_json(self)` in `petstore_api/models/other.py`), so running this example raises `TypeError: to_json() missing 1 required positional argument: 'self'`. Use `other_instance.to_json()`. Root cause is `modules/openapi-generator/src/main/resources/python/model_doc.mustache`, which emits `print({{classname}}.to_json())`; fix that and regenerate so all model docs get the corrected example.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-pydantic-v1/petstore_api/models/tagged_union.py">

<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1/petstore_api/models/tagged_union.py:100">
P2: In this pydantic-v1 sample, `Other` is generated without enforcing the spec's `not` constraint, so `Other.from_json('{"kind": "known"}')` succeeds. This `from_json` therefore counts both subschemas as matching `{"kind": "known"}` and raises "Multiple matches found", even though `known` should deserialize to a `Known` instance; conversely `{"kind": "reserved"}` is accepted as `Other` although the spec excludes it. The pydantic-v2 sample generated in the same PR enforces the exclusion via `kind_validate_not_enum` in `python/petstore_api/models/other.py`. Implement `not` support in the pydantic-v1 generator (mirroring the pydantic-v2 `Other` template) and regenerate, so the pydantic-v1 `TaggedUnion` can deserialize its valid `Known` payload.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx/docs/TaggedUnion.md">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx/docs/TaggedUnion.md:20">
P3: The example calls `to_json()` on the class, but it's an instance method, so `print(TaggedUnion.to_json())` raises `TypeError` when run. Use `print(tagged_union_instance.to_json())`, matching the `to_dict()` call below it. The same pattern appears in the new Known.md and Other.md docs.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/test/test_other.py">

<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/test/test_other.py:47">
P2: These added tests never exercise the `not` keyword this PR claims to cover: `make_instance` consists only of a docstring plus a non-executed string literal, so it always returns `None`, and `testOther` contains no assertions beyond commented-out calls. The suite passes regardless of how `Other` behaves. To deliver the stated coverage, add real assertions (e.g. `Other(kind='unknown')` accepted; `Other(kind='known')`/`Other(kind='reserved')` rejected) instead of regenerating empty stubs.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx-sync/petstore_api/models/tagged_union.py">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx-sync/petstore_api/models/tagged_union.py:57">
P3: `instance` is assigned here but never used in `actual_instance_must_validate_oneof`: every oneOf member is a model type validated by `isinstance`, so the variable is dead code. The `model_oneof.mustache` template emits this line unconditionally, and it is only referenced for container/primitive schemas; for model-typed schemas it should not be emitted. Remove the line from the template and regenerate so this not re-created on the next regeneration.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-pydantic-v1/petstore_api/models/other.py">

<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1/petstore_api/models/other.py:28">
P2: `Other.kind` drops the `not` exclusion from the spec. `oneof-not-enum.yaml` defines `kind` as `not: enum: [known, reserved]`, but this generated model applies no validator, so `Other` accepts `known` and `reserved` (compare `known.py`, which emits `kind_validate_enum`). `TaggedUnion.from_json()` in tagged_union.py counts one match per successful deserialization, so `{"kind": "known"}` now matches both `Known` and `Other` and raises "Multiple matches found" instead of returning a `Known`, and `{"kind": "reserved"}` is wrongly accepted as `Other`. The pydantic-v1 generator never sets the `x-python-not-string-enum-values` extension (AbstractPythonPydanticV1Codegen extends DefaultCodegen, so it skips the `postProcessModelProperty` logic in PythonClientCodegen, and python-pydantic-v1/model_generic.mustache has no `_validate_not_enum` section that python/model_generic.mustache renders), so this regenerated sample does not actually cover the `not` exclusion it was added to test.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx2-sync/test/test_tagged_union.py">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx2-sync/test/test_tagged_union.py:48">
P3: The PR claims to add Python test coverage for the `not` keyword, but the entire test body is commented out: `testTaggedUnion` makes no calls and `make_instance` never constructs an instance, so this file passes trivially and never exercises the `not` semantics that `other.py`'s `kind_validate_not_enum` (rejects `known`/`reserved`) and `known.py` implement. Add a real test that asserts `Other(kind='known')` and `Other(kind='reserved')` raise `ValidationError` and that a valid `Known`/`Other` instance round-trips through `TaggedUnion`.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx2-sync/test/test_known.py">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx2-sync/test/test_known.py:46">
P3: This PR is titled "Add tests for not in python client", but the added test file contains no executable coverage: `testKnown` only has commented-out lines, and `make_instance` returns `None` because its body is also commented out, so the test passes without ever constructing a `Known` or exercising the new `not`-based validation in the companion `Other`/`TaggedUnion` models. Replace the stub body with at least one real assertion (e.g., that `Known(kind='known')` validates and that the excluded-value rejection in `Other` raises) so the sample tests actually guard the `not` behavior the PR claims to cover.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/test/test_tagged_union.py">

<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/test/test_tagged_union.py:47">
P2: This test asserts nothing: `make_instance` has its whole body commented out (and so returns `None` despite the `-> TaggedUnion` annotation), and `testTaggedUnion` runs only commented-out lines. For a PR whose purpose is 'add tests for `not`', the new tests give zero coverage — e.g. they would pass unchanged if `Other` accepted `kind='known'`/'reserved'. Uncomment the body and add real assertions (construct with `actual_instance` — `TaggedUnion(kind='')` as shown in the stub is invalid since the union only takes `actual_instance`).</violation>
</file>

<file name="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/petstore_api/models/tagged_union.py">

<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/petstore_api/models/tagged_union.py:59">
P3: `instance` is assigned via `TaggedUnion.construct()` but never read again in `actual_instance_must_validate_oneof` — the checks only use `v` and `error_messages`. Remove the dead assignment.</violation>

<violation number="2" location="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/petstore_api/models/tagged_union.py:100">
P2: This union cannot perform the `not` matching this PR is meant to cover: `Other` was generated without any `not` validation (`kind: StrictStr` with no validator in `petstore_api/models/other.py`), so `Other.from_json` accepts any `kind` string. As a result `TaggedUnion.from_json('{"kind": "known"}')` succeeds against both schemas, `match` reaches 2, and the method raises "Multiple matches found" instead of deserializing to `Known`; `{"kind": "reserved"}` meanwhile deserializes into `Other` although it should match neither schema. The generator needs to emit the `not` constraint (an enum-excluding validator on `kind`), otherwise the regenerated samples do not reflect the spec and the `not` test coverage is ineffective.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx-sync/docs/Known.md">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx-sync/docs/Known.md:20">
P3: `to_json` is an instance method (`def to_json(self)` in known.py), so `Known.to_json()` raises `TypeError: to_json() missing 1 required positional argument: 'self'`. Use the instance created two lines above: `print(known_instance.to_json())`. The same mistake is generated into docs/Other.md (`print(Other.to_json())`) and docs/TaggedUnion.md (`print(TaggedUnion.to_json())`), rooted in `model_doc.mustache` line 24 (`print({{classname}}.to_json())`); fix the template and regenerate.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-httpx2/docs/TaggedUnion.md">

<violation number="1" location="samples/openapi3/client/petstore/python-httpx2/docs/TaggedUnion.md:20">
P3: The doc example cannot run. `print(TaggedUnion.to_json())` calls `to_json` on the class instead of the instance, so it raises `TypeError: to_json() missing 1 required positional argument: 'self'` ("; the example should print `tagged_union_instance.to_json()`. Earlier in the same snippet `TaggedUnion.from_json("{}")` also raises a ValidationError because `kind` is required, and the placeholder `json = "{}"` shadows the stdlib `json` module. The same broken pattern appears in the newly added `docs/Known.md` and `docs/Other.md`.</violation>
</file>

<file name="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/petstore_api/models/other.py">

<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/petstore_api/models/other.py:28">
P2: The regenerated pydantic-v1 `Other` model silently drops the `not` constraint: `kind` accepts `known`/`reserved` at runtime because no validator is emitted, unlike the enum validator that the same spec produces for `Known` and unlike the default (pydantic-v2) Python client, whose `other.py` emits `kind_validate_not_enum` from the `x-python-not-string-enum-values` handling in `python/model_generic.mustache`. This breaks the PR's stated semantics: `TaggedUnion.from_json('{"kind": "known"}')` now matches both `Known` and `Other` and raises "Multiple matches found" instead of resolving to `Known`, and `{"kind": "reserved"}` incorrectly deserializes to `Other` instead of failing with "No match found". The v1 templates need to emit the `not`-enum validator (as the v2 path does); as generated, the `Other`/`TaggedUnion` test stubs in this sample cannot exercise the intended `not` behavior.</violation>
</file>

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

Re-trigger cubic

)
"""

def testOther(self):

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.

P2: The PR claims to add Python test coverage for the not keyword, but this added test is an empty stub: make_instance always returns None (its body is fully commented out, despite the -> Other annotation), and testOther makes no assertions. Nothing here or in the sibling test_known.py / test_tagged_union.py exercises the generated not validation, so the stated coverage is not actually added. Uncomment make_instance and add assertions that Other accepts a non-excluded value but raises ValueError for known and reserved.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-httpx-sync/test/test_other.py, line 46:

<comment>The PR claims to add Python test coverage for the `not` keyword, but this added test is an empty stub: `make_instance` always returns `None` (its body is fully commented out, despite the `-> Other` annotation), and `testOther` makes no assertions. Nothing here or in the sibling `test_known.py` / `test_tagged_union.py` exercises the generated `not` validation, so the stated coverage is not actually added. Uncomment `make_instance` and add assertions that `Other` accepts a non-excluded value but raises `ValueError` for `known` and `reserved`.</comment>

<file context>
@@ -0,0 +1,52 @@
+        )
+        """
+
+    def testOther(self):
+        """Test Other"""
+        # inst_req_only = self.make_instance(include_optional=False)
</file context>

)
"""

def testKnown(self):

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.

P2: These added "tests" never execute: the make_instance body is a standalone string literal (the second """ block after # uncomment below is an inert string expression) and testKnown only contains commented-out calls, so both pass trivially. For a PR whose stated purpose is Python test coverage for OpenAPI's not keyword (rejecting known/reserved for Other), add executable assertions — e.g. Other(kind='known') raises ValueError, Other(kind='foo') is valid, and TaggedUnion accepts both — so a generator regression in the not handling would fail the suite.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-httpx/test/test_known.py, line 46:

<comment>These added "tests" never execute: the `make_instance` body is a standalone string literal (the second `"""` block after `# uncomment below` is an inert string expression) and `testKnown` only contains commented-out calls, so both pass trivially. For a PR whose stated purpose is Python test coverage for OpenAPI's `not` keyword (rejecting `known`/`reserved` for `Other`), add executable assertions — e.g. `Other(kind='known')` raises ValueError, `Other(kind='foo')` is valid, and `TaggedUnion` accepts both — so a generator regression in the `not` handling would fail the suite.</comment>

<file context>
@@ -0,0 +1,52 @@
+        )
+        """
+
+    def testKnown(self):
+        """Test Known"""
+        # inst_req_only = self.make_instance(include_optional=False)
</file context>

)
"""

def testTaggedUnion(self):

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.

P2: This "test" file, added to give not-keyword coverage, executes nothing: make_instance's body is entirely commented out and testTaggedUnion is commented out, so neither the Other rejection of kind: known/reserved nor the TaggedUnion oneOf matching is ever exercised. make_instance also returns None despite the -> TaggedUnion annotation while it is commented out. The PR description claims Python test coverage for not, but these regenerated test files (in this and all other variants) contain no assertions. Since the samples are regenerated from templates, add real assertions in the generator's Python test templates or a generator unit test that constructs Other(kind='known') / validates TaggedUnion deserialization, rather than shipping inert stubs. Note the commented example is misleading too: kind is not a field of TaggedUnion — the oneOf instance is set via actual_instance/Known/Other.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-httpx-sync/test/test_tagged_union.py, line 46:

<comment>This "test" file, added to give `not`-keyword coverage, executes nothing: `make_instance`'s body is entirely commented out and `testTaggedUnion` is commented out, so neither the `Other` rejection of `kind: known/reserved` nor the `TaggedUnion` oneOf matching is ever exercised. `make_instance` also returns `None` despite the `-> TaggedUnion` annotation while it is commented out. The PR description claims Python test coverage for `not`, but these regenerated test files (in this and all other variants) contain no assertions. Since the samples are regenerated from templates, add real assertions in the generator's Python test templates or a generator unit test that constructs `Other(kind='known')` / validates `TaggedUnion` deserialization, rather than shipping inert stubs. Note the commented example is misleading too: `kind` is not a field of `TaggedUnion` — the oneOf instance is set via `actual_instance`/`Known`/`Other`.</comment>

<file context>
@@ -0,0 +1,52 @@
+        )
+        """
+
+    def testTaggedUnion(self):
+        """Test TaggedUnion"""
+        # inst_req_only = self.make_instance(include_optional=False)
</file context>

error_messages.append(str(e))
# deserialize data into Other
try:
instance.actual_instance = Other.from_json(json_str)

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.

P2: In this pydantic-v1 sample, Other is generated without enforcing the spec's not constraint, so Other.from_json('{"kind": "known"}') succeeds. This from_json therefore counts both subschemas as matching {"kind": "known"} and raises "Multiple matches found", even though known should deserialize to a Known instance; conversely {"kind": "reserved"} is accepted as Other although the spec excludes it. The pydantic-v2 sample generated in the same PR enforces the exclusion via kind_validate_not_enum in python/petstore_api/models/other.py. Implement not support in the pydantic-v1 generator (mirroring the pydantic-v2 Other template) and regenerate, so the pydantic-v1 TaggedUnion can deserialize its valid Known payload.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-pydantic-v1/petstore_api/models/tagged_union.py, line 100:

<comment>In this pydantic-v1 sample, `Other` is generated without enforcing the spec's `not` constraint, so `Other.from_json('{"kind": "known"}')` succeeds. This `from_json` therefore counts both subschemas as matching `{"kind": "known"}` and raises "Multiple matches found", even though `known` should deserialize to a `Known` instance; conversely `{"kind": "reserved"}` is accepted as `Other` although the spec excludes it. The pydantic-v2 sample generated in the same PR enforces the exclusion via `kind_validate_not_enum` in `python/petstore_api/models/other.py`. Implement `not` support in the pydantic-v1 generator (mirroring the pydantic-v2 `Other` template) and regenerate, so the pydantic-v1 `TaggedUnion` can deserialize its valid `Known` payload.</comment>

<file context>
@@ -0,0 +1,141 @@
+            error_messages.append(str(e))
+        # deserialize data into Other
+        try:
+            instance.actual_instance = Other.from_json(json_str)
+            match += 1
+        except (ValidationError, ValueError) as e:
</file context>

)
"""

def testOther(self):

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.

P2: These added tests never exercise the not keyword this PR claims to cover: make_instance consists only of a docstring plus a non-executed string literal, so it always returns None, and testOther contains no assertions beyond commented-out calls. The suite passes regardless of how Other behaves. To deliver the stated coverage, add real assertions (e.g. Other(kind='unknown') accepted; Other(kind='known')/Other(kind='reserved') rejected) instead of regenerating empty stubs.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/test/test_other.py, line 47:

<comment>These added tests never exercise the `not` keyword this PR claims to cover: `make_instance` consists only of a docstring plus a non-executed string literal, so it always returns `None`, and `testOther` contains no assertions beyond commented-out calls. The suite passes regardless of how `Other` behaves. To deliver the stated coverage, add real assertions (e.g. `Other(kind='unknown')` accepted; `Other(kind='known')`/`Other(kind='reserved')` rejected) instead of regenerating empty stubs.</comment>

<file context>
@@ -0,0 +1,53 @@
+        )
+        """
+
+    def testOther(self):
+        """Test Other"""
+        # inst_req_only = self.make_instance(include_optional=False)
</file context>


def testTaggedUnion(self):
"""Test TaggedUnion"""
# inst_req_only = self.make_instance(include_optional=False)

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.

P3: The PR claims to add Python test coverage for the not keyword, but the entire test body is commented out: testTaggedUnion makes no calls and make_instance never constructs an instance, so this file passes trivially and never exercises the not semantics that other.py's kind_validate_not_enum (rejects known/reserved) and known.py implement. Add a real test that asserts Other(kind='known') and Other(kind='reserved') raise ValidationError and that a valid Known/Other instance round-trips through TaggedUnion.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-httpx2-sync/test/test_tagged_union.py, line 48:

<comment>The PR claims to add Python test coverage for the `not` keyword, but the entire test body is commented out: `testTaggedUnion` makes no calls and `make_instance` never constructs an instance, so this file passes trivially and never exercises the `not` semantics that `other.py`'s `kind_validate_not_enum` (rejects `known`/`reserved`) and `known.py` implement. Add a real test that asserts `Other(kind='known')` and `Other(kind='reserved')` raise `ValidationError` and that a valid `Known`/`Other` instance round-trips through `TaggedUnion`.</comment>

<file context>
@@ -0,0 +1,52 @@
+
+    def testTaggedUnion(self):
+        """Test TaggedUnion"""
+        # inst_req_only = self.make_instance(include_optional=False)
+        # inst_req_and_optional = self.make_instance(include_optional=True)
+
</file context>

)
"""

def testKnown(self):

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.

P3: This PR is titled "Add tests for not in python client", but the added test file contains no executable coverage: testKnown only has commented-out lines, and make_instance returns None because its body is also commented out, so the test passes without ever constructing a Known or exercising the new not-based validation in the companion Other/TaggedUnion models. Replace the stub body with at least one real assertion (e.g., that Known(kind='known') validates and that the excluded-value rejection in Other raises) so the sample tests actually guard the not behavior the PR claims to cover.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-httpx2-sync/test/test_known.py, line 46:

<comment>This PR is titled "Add tests for not in python client", but the added test file contains no executable coverage: `testKnown` only has commented-out lines, and `make_instance` returns `None` because its body is also commented out, so the test passes without ever constructing a `Known` or exercising the new `not`-based validation in the companion `Other`/`TaggedUnion` models. Replace the stub body with at least one real assertion (e.g., that `Known(kind='known')` validates and that the excluded-value rejection in `Other` raises) so the sample tests actually guard the `not` behavior the PR claims to cover.</comment>

<file context>
@@ -0,0 +1,52 @@
+        )
+        """
+
+    def testKnown(self):
+        """Test Known"""
+        # inst_req_only = self.make_instance(include_optional=False)
</file context>

# create an instance of Known from a JSON string
known_instance = Known.from_json(json)
# print the JSON string representation of the object
print(Known.to_json())

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.

P3: to_json is an instance method (def to_json(self) in known.py), so Known.to_json() raises TypeError: to_json() missing 1 required positional argument: 'self'. Use the instance created two lines above: print(known_instance.to_json()). The same mistake is generated into docs/Other.md (print(Other.to_json())) and docs/TaggedUnion.md (print(TaggedUnion.to_json())), rooted in model_doc.mustache line 24 (print({{classname}}.to_json())); fix the template and regenerate.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-httpx-sync/docs/Known.md, line 20:

<comment>`to_json` is an instance method (`def to_json(self)` in known.py), so `Known.to_json()` raises `TypeError: to_json() missing 1 required positional argument: 'self'`. Use the instance created two lines above: `print(known_instance.to_json())`. The same mistake is generated into docs/Other.md (`print(Other.to_json())`) and docs/TaggedUnion.md (`print(TaggedUnion.to_json())`), rooted in `model_doc.mustache` line 24 (`print({{classname}}.to_json())`); fix the template and regenerate.</comment>

<file context>
@@ -0,0 +1,29 @@
+# create an instance of Known from a JSON string
+known_instance = Known.from_json(json)
+# print the JSON string representation of the object
+print(Known.to_json())
+
+# convert the object into a dict
</file context>
Suggested change
print(Known.to_json())
print(known_instance.to_json())

# create an instance of TaggedUnion from a JSON string
tagged_union_instance = TaggedUnion.from_json(json)
# print the JSON string representation of the object
print(TaggedUnion.to_json())

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.

P3: The doc example cannot run. print(TaggedUnion.to_json()) calls to_json on the class instead of the instance, so it raises TypeError: to_json() missing 1 required positional argument: 'self' ("; the example should print tagged_union_instance.to_json(). Earlier in the same snippet TaggedUnion.from_json("{}") also raises a ValidationError because kind is required, and the placeholder json = "{}" shadows the stdlib json module. The same broken pattern appears in the newly added docs/Known.md and docs/Other.md.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-httpx2/docs/TaggedUnion.md, line 20:

<comment>The doc example cannot run. `print(TaggedUnion.to_json())` calls `to_json` on the class instead of the instance, so it raises `TypeError: to_json() missing 1 required positional argument: 'self'` ("; the example should print `tagged_union_instance.to_json()`. Earlier in the same snippet `TaggedUnion.from_json("{}")` also raises a ValidationError because `kind` is required, and the placeholder `json = "{}"` shadows the stdlib `json` module. The same broken pattern appears in the newly added `docs/Known.md` and `docs/Other.md`.</comment>

<file context>
@@ -0,0 +1,29 @@
+# create an instance of TaggedUnion from a JSON string
+tagged_union_instance = TaggedUnion.from_json(json)
+# print the JSON string representation of the object
+print(TaggedUnion.to_json())
+
+# convert the object into a dict
</file context>
Suggested change
print(TaggedUnion.to_json())
print(tagged_union_instance.to_json())


@validator('actual_instance')
def actual_instance_must_validate_oneof(cls, v):
instance = TaggedUnion.construct()

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.

P3: instance is assigned via TaggedUnion.construct() but never read again in actual_instance_must_validate_oneof — the checks only use v and error_messages. Remove the dead assignment.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At samples/openapi3/client/petstore/python-pydantic-v1-aiohttp/petstore_api/models/tagged_union.py, line 59:

<comment>`instance` is assigned via `TaggedUnion.construct()` but never read again in `actual_instance_must_validate_oneof` — the checks only use `v` and `error_messages`. Remove the dead assignment.</comment>

<file context>
@@ -0,0 +1,141 @@
+
+    @validator('actual_instance')
+    def actual_instance_must_validate_oneof(cls, v):
+        instance = TaggedUnion.construct()
+        error_messages = []
+        match = 0
</file context>

@wing328
wing328 merged commit 13fc20c into master Oct 5, 2026
61 checks passed
@wing328
wing328 deleted the python-test-not branch October 5, 2026 10:19
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.

1 participant