Generation test case - #470
Open
brandon-wada wants to merge 4 commits into
Open
Conversation
`make generate` produces a small diff against the committed tree, so `generated/` had been hand-edited rather than regenerated (the datamodel-codegen header still read `timestamp: 2026-07-10`, from before the API-token fields were added in #445). Nothing was functionally wrong; the diff is docstrings and formatting: - generated/model.py: black splits the two long `description=` strings on the ApiToken/ApiTokenCreateResponse `last_used_at` and `token_ttl` fields. - api_token.py, api_token_create_response.py: the `token_ttl` argument was missing from the `__init__`/`_from_openapi_data` docstrings, and `last_used_at`'s docstring still had the pre-nullable type and a truncated description. This is the output of running `make generate` and nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`generated/` is excluded from linting (LINT_PATHS), so nothing caught hand-edits to generated code. The cost shows up later: the next person to run `make generate` gets a large unexplained diff mixed into their own PR. test_codegen/ re-runs each half of `make generate` into a scratch tree and diffs it against what is committed, failing with the actual diff and naming `make generate`: - datamodel-codegen (generated/model.py) is pure Python and already a dependency, so that half always runs. Its `timestamp:` header line varies per run and is normalized. - openapi-generator-cli needs node and java, so that half skips with a reason when they are missing -- except when REQUIRE_OPENAPI_GENERATOR is set, which the new test-codegen CI job does, so the check cannot quietly become a no-op where it is the only place it reliably runs. Notes on why the tests look the way they do: - They live outside test/ because test/conftest.py builds a Groundlight() client in pytest_configure, so everything under test/ needs GROUNDLIGHT_API_TOKEN to collect. These checks need no API access, and requiring a token would mean they never ran. `make test` depends on the new `make test-codegen` target, so a contributor running the suite gets them without knowing they exist. - The scratch tree has to be inside the repo: datamodel-codegen (which formats its own output) and black both find our [tool.black] settings by walking up from the paths they write, so generating into /tmp silently yields black's default 88-column style. - The openapi half regenerates over a *copy* of the committed tree rather than into an empty directory, because openapi-generator leaves existing files alone (the stubs under generated/test/) and lists only what it wrote in .openapi-generator/FILES. - The generator command lines are parsed out of the Makefile rather than duplicated, so changing how the SDK is generated cannot leave these tests checking the old way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The job failed on `node-version: 21` with ERR_REQUIRE_ESM: openapi-generator-cli 2.x `require()`s proxy-agent, which is ESM-only, and node 21 has no require(esm) support. The docs jobs pin 21 happily because nothing there loads the generator. Reproduced locally against this exact spec and node_modules, with `REQUIRE_OPENAPI_GENERATOR=1 make test-codegen`: node 18.19.1 ERR_REQUIRE_ESM (fails, same stack as CI) node 20.19.0 passes node 21.7.3 ERR_REQUIRE_ESM (fails, same stack as CI) node 22.14.0 passes which matches require(esm) landing in 20.19 and 22.12 and never in 21.x. Pinned the job to 22 and wrote the requirement into DEVELOPING.md, since `make generate` itself fails the same way for anyone on node 21. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a test to check that the generated folder is actually generated
Note: I'm testing out a new agent orchestration tool with this. Any human should please point out anything funny that arose from this