Skip to content

Fix C# listFiles E2E ordering assumption#1261

Merged
stephentoub merged 2 commits into
mainfrom
stephentoub/fix-csharp-e2e-order
May 12, 2026
Merged

Fix C# listFiles E2E ordering assumption#1261
stephentoub merged 2 commits into
mainfrom
stephentoub/fix-csharp-e2e-order

Conversation

@stephentoub
Copy link
Copy Markdown
Collaborator

@stephentoub stephentoub commented May 12, 2026

The C# and Rust E2E tests assumed workspaces.listFiles returns files in sorted or stable order, but the runtime does not guarantee workspace file enumeration order. That made the tests flaky when created files came back in a different filesystem/RPC order.

This updates both tests to assert the supported behavior instead: each listFiles call includes all files created by the test without asserting the returned order. The assertions compare the filtered results after ordinal sorting in C# and use presence checks for each Rust call.

Validation:

  • dotnet test dotnet\test\GitHub.Copilot.SDK.Test.csproj --filter "FullyQualifiedName~Workspaces_CreateFile_Then_ListFiles_Returns_All_Files" --logger "console;verbosity=minimal"
  • cargo +nightly-2026-04-14 fmt --check && cargo test --features test-support --test e2e workspaces_createfile_then_listfiles_returns_all_files -- --nocapture

Copilot AI review requested due to automatic review settings May 12, 2026 00:57
@stephentoub stephentoub requested a review from a team as a code owner May 12, 2026 00:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a flaky .NET E2E test that incorrectly assumed workspaces.listFiles returns files in a stable/sorted order. The test now validates the supported contract: all created files are present, without asserting enumeration order.

Changes:

  • Renamed the E2E test to reflect the intended assertion (all files returned, not order).
  • Updated assertions to compare results after ordinal sorting, avoiding filesystem/RPC enumeration-order dependence.
  • Updated comments to document that ordering is not guaranteed and to clarify the repeated-call expectation.
Show a summary per file
File Description
dotnet/test/E2E/RpcAdditionalEdgeCasesE2ETests.cs Makes the ListFiles E2E test order-independent by sorting filtered results before asserting equality.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR fixes both the .NET and Rust E2E tests consistently — the same ordering assumption was present in both, and both are corrected in parallel with equivalent logic. The fix pattern is symmetric:

SDK Before After
.NET Assert.Equal(paths, matchingFiles) (sorted assumption) Assert.Equal(paths, matchingFiles.OrderBy(...))
Rust assert_eq!(first.files, second.files) (stable assumption) assert!(files.iter().any(...)) for each file

The other SDKs do not have this specific "sorted or stable order" assertion — their listFiles tests already use order-independent checks (toContain in Node.js, in in Python, containsString in Go), so no changes are needed there.

No cross-SDK consistency issues found.

Generated by SDK Consistency Review Agent for issue #1261 · ● 342.2K ·

@stephentoub stephentoub merged commit 9915343 into main May 12, 2026
25 checks passed
@stephentoub stephentoub deleted the stephentoub/fix-csharp-e2e-order branch May 12, 2026 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants