Skip to content

Create the schema export directory when it does not exist - #10369

Merged
glen-84 merged 1 commit into
mainfrom
gai/create-schema-export-directory
Sep 8, 2026
Merged

glen-84 merged 1 commit into
mainfrom
gai/create-schema-export-directory

Conversation

@glen-84

@glen-84 glen-84 commented Sep 8, 2026

Copy link
Copy Markdown
Member

Summary

  • SchemaFileExporter.Export now creates the target directory before writing the schema and settings files. The previous guard only called Directory.CreateDirectory when the directory already existed, so exporting into a path that did not exist yet failed with DirectoryNotFoundException.
  • The guard keeps skipping an empty directory component, which is what a bare relative file name such as schema.graphqls yields and what Directory.CreateDirectory rejects.

Test plan

  • New Export_Should_CreateDirectory_When_DirectoryDoesNotExist exports into a nested path that does not exist and asserts both files are written. It failed with DirectoryNotFoundException before the change.
  • The two existing SchemaFileExporterTests no longer pre-create the directory themselves and still pass.

Copilot AI lite review requested due to automatic review settings September 8, 2026 08:58

Copilot AI 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.

🔵 Needs a closer look

The new directory guard can throw for bare relative filenames because Path.GetDirectoryName may return null, causing a regression.

Pull request overview

This PR fixes SchemaFileExporter.Export so exporting a schema to a path whose directory doesn’t exist no longer fails, by creating the target directory before writing the schema and settings files.

Changes:

  • Update SchemaFileExporter.Export to create the target directory when a non-empty directory component is present.
  • Adjust existing exporter tests to stop pre-creating the directory.
  • Add a new test to validate exporting into a nested (previously non-existent) directory.
File summaries
File Description
src/HotChocolate/Core/src/Types/Execution/Internal/SchemaFileExporter.cs Create export directory before writing output files (but needs null-safe directory handling).
src/HotChocolate/Core/test/Types.Tests/Execution/Internal/SchemaFileExporterTests.cs Update tests to rely on exporter directory creation; add nested-directory regression test.
Review details

Suppressed comments (1)

src/HotChocolate/Core/src/Types/Execution/Internal/SchemaFileExporter.cs:48

  • Path.GetDirectoryName(schemaFileName) can return null for a bare relative filename (e.g. "schema.graphqls"). With the new directory.Length > 0 guard and Path.Combine(directory, ...), this will throw (NullReferenceException / ArgumentNullException). Coalesce null to an empty string before the length check and combine so the original behavior (writing to the current directory) remains valid.
        var directory = System.IO.Path.GetDirectoryName(schemaFileName)!;

        if (directory.Length > 0)
        {
            Directory.CreateDirectory(directory);
        }

        var baseName = System.IO.Path.GetFileNameWithoutExtension(schemaFileName);
        var settingsFileName = System.IO.Path.Combine(directory, $"{baseName}-settings.json");

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Patch coverage

100.0% of changed lines covered (1/1)

File Covered Changed Patch %
…/Core/src/Types/Execution/Internal/SchemaFileExporter.cs 1 1 100.0% 🟢

Project coverage: 57.9% (288135/497753 lines)

@glen-84
glen-84 merged commit 78361e5 into main Sep 8, 2026
152 checks passed
@glen-84
glen-84 deleted the gai/create-schema-export-directory branch September 8, 2026 09:07
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.

2 participants