Skip to content

Fix tests that fail when running all target frameworks - #10435

Merged
glen-84 merged 1 commit into
mainfrom
gai/multi-tfm-test-failures
Sep 25, 2026
Merged

glen-84 merged 1 commit into
mainfrom
gai/multi-tfm-test-failures

Conversation

@glen-84

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

Copy link
Copy Markdown
Member

Summary

CI tests net11.0 only, so these failures show up only when the suite runs on every target framework.

  • Spatial tests on net8.0. PostgreSqlResource.RunSqlScriptAsync opens its connection before the script runs, so Npgsql caches the database's types for that connection string without the ones the script creates (geometry, from CREATE EXTENSION postgis). EF Core 8 reuses that cache and fails with The NpgsqlDbType 'Geometry' isn't present in your database. RunSqlScriptAsync now calls ReloadTypes() after the script. The stale cache exists in every Npgsql version, so the reload runs on all target frameworks. It uses the synchronous overload because the resource compiles against Npgsql 8.0.7, while 9.0.x and 10.0.0 to 10.0.2 only have ReloadTypesAsync(CancellationToken). Squadron ran scripts through psql inside the container, so this started with Replace Squadron with CookieCrumble Testcontainers resources #10338.
  • IsProjectedSelectorTests on net8.0 and net9.0. EF Core 8 and 9 translate the DataLoader key lookup to IN (SELECT ... json_each(...)), so the test now has that SQL as the inline snapshot for those target frameworks.
  • CookieCrumble Match_StrictMode_Off flake. The test creates and deletes snapshot files whose names were shared by every target framework's test process, so one process could read a file another had just written. The global formatter registered by SnapshotBuilder_Segment_Custom_Global_Serializer made the contents differ when the two processes ran the tests in a different order. The file names now include the target framework.

Test plan

  • Both Spatial test projects, IsProjectedSelectorTests, and the CookieCrumble tests pass on net8.0, net9.0, net10.0, and net11.0.
  • With plain Npgsql 8.0.7 and 10.0.0, a new connection opened after the script reads geometry as an unknown type, and as public.geometry after ReloadTypes().
  • The CookieCrumble failure reproduced every time by pre-creating another target framework's snapshot file before the fix. With per-framework file names, the test no longer reads it.

Copilot AI lite review requested due to automatic review settings September 25, 2026 17:03

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.

Copilot review overview

🟢 Approval recommended

No unresolved issues were identified that would block approval.

Review effort: Lite
Findings: None

What changed in this PR

Updates multi-target test infrastructure so tests pass consistently on .NET 8–11.

Changes:

  • Refreshes PostgreSQL type metadata after SQL scripts.
  • Adds framework-specific EF Core SQL snapshots.
  • Isolates CookieCrumble snapshots by target framework.
File Description
src/​HotChocolate/​Data/​test/​Data.EntityFramework.Tests/​IsProjectedSelectorTests.cs Updated as part of this pull request.
src/​CookieCrumble/​test/​CookieCrumble.Tests/​SnapshotTests.cs Updated as part of this pull request.
src/​CookieCrumble/​src/​CookieCrumble.Resources.PostgreSql/​PostgreSqlResource.cs Updated as part of this pull request.

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

@github-actions

Copy link
Copy Markdown
Contributor

Patch coverage

100.0% of changed lines covered (1/1)

File Covered Changed Patch %
…/src/CookieCrumble.Resources.PostgreSql/PostgreSqlResource.cs 1 1 100.0% 🟢

Project coverage: 58.9% (300422/510417 lines)

@glen-84
glen-84 merged commit 4c73df4 into main Sep 25, 2026
159 checks passed
@glen-84
glen-84 deleted the gai/multi-tfm-test-failures branch September 25, 2026 17:45
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