test(core): add regression coverage for cross-file $ref schema exports (#1107) - #3381
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a test fixture and regression test for Issue 1107 to ensure Orval correctly exports interfaces and type aliases when OpenAPI schemas are referenced via cross-file external ChangesIssue 1107 Cross-file $ref Test Fixture
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a regression test for issue #1107, ensuring that top-level components.schemas.X entries that are themselves cross-file $refs generate properly exported type aliases.
Changes:
- New OpenAPI specs (
issue-1107.yaml+ externalissue-1107-petstore.yaml) reproducing the cross-file$refscenario. - New Orval config entry
issue-1107-cross-file-refplus a focused test asserting exports in generated model files. - Snapshot fixtures for the expected generated endpoints and model output.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/specifications/issue-1107/issue-1107.yaml | Main spec referencing external schemas via $ref. |
| tests/specifications/issue-1107/issue-1107-petstore.yaml | External spec defining Pet, Pets, Error. |
| tests/configs/default.config.ts | Registers the new generation target. |
| tests/api-generation.spec.ts | Adds regression test asserting exports for cross-file $ref aliases. |
| tests/snapshots/default/issue-1107-cross-file-ref/model/pets.ts | Expected Pets alias output. |
| tests/snapshots/default/issue-1107-cross-file-ref/model/pet.ts | Expected Pet interface output. |
| tests/snapshots/default/issue-1107-cross-file-ref/model/index.ts | Expected model barrel. |
| tests/snapshots/default/issue-1107-cross-file-ref/model/error.ts | Expected Error interface output. |
| tests/snapshots/default/issue-1107-cross-file-ref/endpoints.ts | Expected endpoints output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(await model('error.ts')).toContain('export interface Error {'); | ||
|
|
There was a problem hiding this comment.
Leaving as-is. export interface Error shadowing the global is pre-existing, generator-wide behavior — orval emits any schema named Error this way, independent of cross-file $ref. The original #1107 reproduction uses Error too, so keeping the name makes the fixture faithful to the issue. Renaming/suffixing to avoid global collisions is a separate generator concern, out of scope for this test-only PR.
| expect(content).toContain('headers = unref(headers);'); | ||
| }); | ||
|
|
||
| test('default issue-1107 emits exports for schemas defined via cross-file $ref', async () => { |
There was a problem hiding this comment.
This follows the established pattern in this file. generated/ is produced by bun run generate-api before test:snapshots (see the npm scripts and CI), and every existing focused test here (issue-708/826/873/1026) reads from generated/ the same way — a per-test beforeAll would be inconsistent with the suite. The snapshots aren't unused either: describeApiGenerationSnapshots (test-utils/snapshot-testing.ts) scans generated/default/** and snapshot-compares every file, including issue-1107-cross-file-ref. The focused test is an additional targeted assertion on top of that.
| Pet: | ||
| $ref: './issue-1107-petstore.yaml#/components/schemas/Pet' |
There was a problem hiding this comment.
Summary
Adds regression coverage for #1107 ("Type alias exports missing when using cross-file
$ref").Fix #1107 reported that a top-level
components.schemas.Xdefined as a cross-file$ref(X→ another file'sX) generated a schema file containing the import but noexportfor the type — a dangling, unusable module.I could not reproduce this on current
master(v8.11.0): cross-file$refschemas now resolve correctly and every referenced schema is emitted as a usable exported type. The issue appears to have been resolved as a side effect of the multi-file handling rework, but there was no test guarding the scenario, so a future change could silently reintroduce it.This PR is test-only — it locks in the current correct behavior.
What's added
tests/specifications/issue-1107/— a spec whose top-levelcomponents.schemas.{Pet,Pets,Error}are cross-file$refs into a second file, generated with split schemas (output.schemas).issue-1107-cross-file-refentry intests/configs/default.config.ts, plus its generated snapshot.tests/api-generation.spec.tsverifying each referenced schema file emits itsexport(export interface Pet,export interface Error, andimport type { Pet }+export type Pets = Pet[]), so a regression fails with a targeted message rather than only a full-file snapshot diff.The focused test was confirmed to fail when the
exportline is removed from the generated output, so it genuinely guards the #1107 behavior.Notes
$refs. Those are intentionally left out here: the regression target of Type alias exports missing when using cross-file $ref #1107 is cross-file schema$ref, and external path-item$refcurrently fails with a separate, unrelated error (filed as External path-item $ref fails with "Can't resolve reference" (JSON Pointer escapes not decoded) #3380). Keeping this test schema-focused keeps it independent of that bug.format:check,lint,typecheck,test,test:snapshots, generated typecheck, and mock verification.Summary by CodeRabbit
Tests
Chores