Fix CI silently skipping the integration test suite - #603
Merged
Merged
Conversation
…ownload, and add PR preview publishing (#570) Restore the indentation of IntrospectionUpdateTests.fs The file was re-indented so that `task { ... }` bodies became offside (`let options` stayed at the outer column while the surrounding lines moved in). Both SDK 10.0.303 and 10.0.401 reject it with FS0010/FS0058, so the integration test project has not compiled since. CI stayed green only because it never built the project. The original change was whitespace-only, so this restores the previous version of the file verbatim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ownload, and add PR preview publishing (#570) Build the integration tests before running them, fail on empty runs `UpdateIntrospectionFile` switched to `dotnet test --no-build`, but nothing restores or builds FSharp.Data.GraphQL.Integration.slnx: the restore target only covers `src/**` and the build target only builds FSharp.Data.GraphQL.slnx. On an unrestored project the test SDK props are not imported, so `dotnet test` runs nothing and exits with 0. The step has been a silent no-op on CI, which let a non-compiling integration test project stay green. - Add a `BuildIntegrationTests` target that builds the integration solution after `Build` (it needs the client provider assembly). - Make `runTests` fail when the run leaves no results file, deleting a stale one first so an earlier run cannot mask it. - Reuse `runTests` for `UpdateIntrospectionFile` instead of repeating the `dotnet test` options, with its own results file name so it does not overwrite other runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ionFactory` and remove external server orchestration from build (#564) Run the whole integration test suite again The migration to in-process hosts dropped the `RunIntegrationTests` target and left only `UpdateIntrospectionFile`, which filters to `IntrospectionUpdateTests`. The client provider, operation error and reserved scalar tests have not been run by the build pipeline since. Restore a `RunIntegrationTests` target after `UpdateIntrospectionFile` that runs every other integration test into its own results file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SwapiLocalProviderTests and SwapiRemoteProviderTests both instantiate
`GraphQLProvider<"introspection.json">` and both declare inline
operations named `Q` and `M`. With the F# compiler from SDK 10.0.3xx,
which CI is pinned to, a clean build of the integration tests then fails
with FS0193 ("type ...Operations.Q.OperationResult is not compatible
with type ...Operations.Q.OperationResult"). SDK 10.0.401 compiles it,
and reverting FSharp.TypeProviders.SDK to 8.1.0 does not help, so this
is a compiler issue rather than a change in this repository. It went
unnoticed because CI stopped building the integration tests.
Rename the remote operations to `RemoteQ` and `RemoteM`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The pipeline now builds and executes all test suites while reliably detecting silent test-run failures.
Pull request overview
Restores reliable CI coverage for the integration test suite and prevents silent no-op test runs.
Changes:
- Builds and runs integration tests in the FAKE pipeline.
- Fails test runs that produce no TRX results.
- Fixes test indentation and provider operation-name collisions.
File summaries
| File | Description |
|---|---|
build/Program.fs |
Adds integration build/test targets and result validation. |
tests/FSharp.Data.GraphQL.IntegrationTests/IntrospectionUpdateTests.fs |
Restores valid F# indentation. |
tests/FSharp.Data.GraphQL.IntegrationTests/SwapiRemoteProviderTests.fs |
Uses unique generated operation names. |
Review details
- Files reviewed: 2/3 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Test Results 9 files 9 suites 11m 15s ⏱️ Results for commit bdada17. |
stanislavigertrud
approved these changes
Sep 15, 2026
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
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.
CI has been building
FSharp.Data.GraphQL.slnxand running only the unit tests.FSharp.Data.GraphQL.Integration.slnx(which containsIntegrationTests/IntegrationTests.Server) is never restored or built, soUpdateIntrospectionFile'sdotnet test --no-buildruns against a project without an imported test SDK: it prints "Build succeeded" and exits 0 without running anything. That silent no-op let two real breaks sit ondevunnoticed:IntrospectionUpdateTests.fshas had broken indentation (FS0010/FS0058) since Publish CI test results into PR discussions, harden artifact download, and add PR preview publishing #570 - the project has not compiled since then.RunIntegrationTestswas dropped entirely by Migrate integration tests to in-process hosts viaWebApplicationFactoryand remove external server orchestration from build #564, along with the server-orchestration code it replaced, soLocalProviderTests,SwapiLocalProviderTests,OperationErrorTests, etc. have not run in CI since.Changes
fixup!Publish CI test results into PR discussions, harden artifact download, and add PR preview publishing #570: restore the indentation ofIntrospectionUpdateTests.fs(whitespace-only fix, reverts the file to its pre-Publish CI test results into PR discussions, harden artifact download, and add PR preview publishing #570 content).fixup!Publish CI test results into PR discussions, harden artifact download, and add PR preview publishing #570: add aBuildIntegrationTeststarget that buildsFSharp.Data.GraphQL.Integration.slnxafterBuild; makerunTestsdelete any stale results file first and fail when the run leaves no results file, so an unbuilt/unrestored project can no longer pass silently;UpdateIntrospectionFilenow goes throughrunTestsinstead of duplicating itsdotnet testoptions.fixup!Migrate integration tests to in-process hosts viaWebApplicationFactoryand remove external server orchestration from build #564: restore aRunIntegrationTeststarget that runs every integration test other thanIntrospectionUpdateTests(which already ran inUpdateIntrospectionFile).SwapiRemoteProviderTests.fs(Q/M→RemoteQ/RemoteM). Once the integration tests actually got built, a second hidden break surfaced: on the SDK 10.0.3xx line this repo is pinned to (not on 10.0.401), the F# compiler fails with FS0193 ("type X is not compatible with type X") when two files instantiate the sameGraphQLProvider<"introspection.json">and declare inline operations with the same name - hereSwapiLocalProviderTests.fsandSwapiRemoteProviderTests.fs. RevertingFSharp.TypeProviders.SDK8.10.0 → 8.1.0 did not help, so this looks like a compiler issue rather than something introduced by a repo change; renaming the operations works around it.Verification
Ran the full
BuildAndTestFAKE pipeline from a clean worktree on SDK 10.0.303 (matching CI):Build,RunUnitTests(601 tests),BuildIntegrationTests,UpdateIntrospectionFile(2 tests),RunIntegrationTests(103 tests) all succeeded, producing three separate.trxfiles. Also verified the negative case: with the integration project deliberately left unbuilt,UpdateIntrospectionFilenow fails with "produced no test results ... Was the project restored and built?" instead of silently exiting 0.Notes for review
#570and#564are already ondev, so thesefixup!commits can't be autosquashed onto them withgit rebase --autosquash- they'd need to be squash-merged or combined by hand.fixup!commit for Migrate integration tests to in-process hosts viaWebApplicationFactoryand remove external server orchestration from build #564 depends on therunTestssignature change made in thefixup!commit for Publish CI test results into PR discussions, harden artifact download, and add PR preview publishing #570 (order matters if squashing).build/Program.fsstill has unusedBuildIntegrationTestServer/StartIntegrationServer/StopIntegrationServertargets left over from before Migrate integration tests to in-process hosts viaWebApplicationFactoryand remove external server orchestration from build #564 removed their only caller.🤖 Generated with Claude Code