Make the ComponentTests checker tests asynchronous - #20461
Conversation
Tests that blocked on Async.RunSynchronously now return Task and bind the FCS Async results inside task { }: SemanticClassificationRegressions, FindReferences, CommonWorkflows, TransparentCompiler (Fuzzing, getParseResult), CompilerDirectives/Line, Miscellaneous/TestUtilities, CompilationFromCmdlineArgsTests and AsyncMemoize, plus the singleFileChecker helper in FSharp.Test.Utilities. Async.RunSynchronously inside the F# sources under test, the graph tests' parseSourceCode and AsyncMemoize's Stress test are unchanged.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
✅ No release notes required |
ComponentTests checker tests asynchronous
xperiandri
left a comment
There was a problem hiding this comment.
Reviewed cd3849f as a full diff plus the untouched callers of every helper whose shape changed.
Verdict: the conversion is sound; nothing blocking. Two inline remarks — one fold-the-duplicate (non-blocking, but this PR is the natural place for it), one nit.
What was checked:
- Every caller of a helper that changed shape (
singleFileChecker,parse,getParseResult,findRefsInSource,getClassifications,classificationsForIdent,waitUntil/expect/assertTaskCanceled) is converted, and none exists outsideFSharp.Compiler.ComponentTests.Stress testand the two pre-existingtasktests inAsyncMemoize.fsuse none of them, so leaving them alone is right. let!/do!on anAsyncinsidetask { }goes throughAsync.StartImmediateAsTaskwith the default cancellation token — the same token and exception propagationAsync.RunSynchronouslyhad — so there is no semantic drift, including thetry … finallyaroundEnvironment.CurrentDirectoryinCompilationFromCmdlineArgsTests(both theories are skipped anyway).- Source snippets in triple-quoted strings keep their columns; the UTF-8 BOM on the five files that had one is preserved;
git diff --checkis clean. - Ran the touched modules locally against a Debug/net11.0 build of this commit:
AsyncMemoize,SemanticClassificationRegressions,FindReferences,CommonWorkflows,CompilerDirectives.Line,Miscellaneous.TestUtilities— 128 passed, 2 skipped, 0 failed.FuzzingisSkipped on main, so its conversion is compile-verified only.
Non-blocking observation: the explicit : Task return annotation is new to this project — the existing task { } tests, including two in AsyncMemoize.fs and those in TransparentCompiler.fs, rely on the inferred Task<unit>. It is a good guard (an accidentally returned Async<unit> becomes a compile error instead of a value xunit has no way to await), so I would keep it; the two neighbours in AsyncMemoize.fs could get it too for uniformity, or not — either is fine.
Review follow-up: Helpers.singleFileCheckerWithName takes the file name and singleFileChecker is its test.fs case, so the module-local copy in FindReferences.fs goes away; findRefsInFile/testFindRefsInFile carry the name and the #line test becomes a one-liner. assertTaskCanceled's parameter is job, not task, and the two remaining task tests in AsyncMemoize.fs get the same : Task annotation as the rest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Description
Test-only change. The checker tests in
FSharp.Compiler.ComponentTeststhat blocked onAsync.RunSynchronouslynow returnTaskand bind the FCSAsyncresults insidetask { }, so they no longer park an xunit worker thread while the checker runs, and they read like the Task-based tests already in the project.FSharpChecker/SemanticClassificationRegressions.fs—getClassifications,classificationsForIdentandexpectUnionCaseClassificationsare asynchronous; every test binds them withlet!. Source snippets keep their exact text and columns, only the code around them moved.FSharpChecker/FindReferences.fs—findRefsInSource,testFindRefsInSourceandsingleFileCheckerWithNameare asynchronous; the tests that call them return the resultingTask.FSharpChecker/CommonWorkflows.fs,FSharpChecker/TransparentCompiler.fs(Fuzzing,getParseResult),CompilerDirectives/Line.fs,Miscellaneous/TestUtilities.fs,TypeChecks/Graph/CompilationFromCmdlineArgsTests.fs— the same, one site each.CompilerService/AsyncMemoize.fs—waitUntil,expectandassertTaskCanceledreturnAsync/Taskand the tests await them;.Result/.Wait()on the tasks those tests create becamelet!/do!too.Stress testis untouched: itsTask.Waitwith a timeout is the point of the test.FSharp.Test.Utilities/ProjectGeneration.fs—singleFileCheckerreturnsAsync; its only callers are the files above.Left as is:
Async.RunSynchronouslyinside the F# source strings that the tests compile or run (the behaviour under test),TypeChecks/Graph/Utils.fs'sparseSourceCode(called from record literals in 30 places across the trie/graph tests; a separate change), andFSharp.Compiler.Service.Tests, which has a few hundred more sites.Checklist