More regression testing into the CI #19260
Open
+487
−77
Conversation
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
Contributor
✅ No release notes required |
Member
Author
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Member
Author
|
/azp run |
|
Azure Pipelines failed to run 2 pipeline(s). |
The repo 'demystifyfp/FsToolkit.ErrorHandling' contains a period which is not allowed in Azure DevOps job identifiers (alphanumeric + underscore only). This caused YAML validation failure preventing the entire pipeline from starting. Add an additional replace() call for '.' -> '_'.
Three fixes: 1. Checkout step: split buildScript to check only the file name (first token), not the entire string including arguments 2. Build step: use 'cmd /c' on Windows for file-based scripts to handle arguments naturally; on Linux, chmod only the script file 3. OpenTK: use single project in dotnet build (MSB1008 error with multiple projects)
…mands 1. Add .NET 9.0.x SDK installation - IcedTasks' FAKE build project targets net9.0 and fails without the runtime 2. Switch IcedTasks and FsToolkit from FAKE build.cmd to direct dotnet build/test - FAKE's build.exe locks itself causing MSB3027 file lock errors and cascading FS0229 metadata read failures 3. Keep .NET 9.0 SDK for repos that may need it
IcedTasks' Directory.Build.targets runs 'dotnet tool restore' during build. When MSBuild builds projects in parallel, concurrent tool restores fight over NuGet package files causing file lock errors. Pre-restoring tools eliminates the race condition.
…plicate type name conflict Instance extension members compile with the extended type as the first IL parameter, so they can never produce duplicate IL method signatures even when the simple type name matches. Only static extension members lack this distinguishing parameter. This fixes a regression where libraries like IcedTasks that define instance (inline) extension members on builder types from different namespaces (e.g. Microsoft.FSharp.Control.TaskBuilderBase and IcedTasks.TaskBase.TaskBuilderBase) were incorrectly rejected. Regression introduced by PR #18821 (commit e948a68).
The regression test template now supports multiple commands in a single matrix entry using ';;' as a separator. Commands run sequentially and the job fails on the first non-zero exit code. Example: buildScript: dotnet build A.fsproj ;; dotnet build B.fsproj Reverts OpenTK back to a single matrix entry with both test projects.
…Nullness disabled When langFeatureNullness is false, p_ty2 conditionally skipped writing nullness B-bytes for type tags 1-4, but u_ty unconditionally reads them. This caused B-stream misalignment when constraint data (e.g. NotSupportsNull from BCL types) was also written to B-stream, leading to FS0229 errors when consuming metadata. Fix: Always write a B-byte (0 = AmbivalentToNull) for each type tag, regardless of langFeatureNullness. This keeps streams aligned. Also adds ImportTests.fs to the test project (was missing since migration) and adds two regression tests for the B-stream misalignment scenario.
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
…stance members The duplicate extension member check (FS3356) only applies to static extension members because they lack the extended type as a first IL parameter, causing signature collisions. Instance extension members are safe because the extended type differentiates the IL signatures. Updated the existing tests to use static members (the actual IL collision case) and added companion tests showing instance members on same-named types are allowed.
Member
Author
|
/run fantomas |
Contributor
🔧 CLI Command Report
✅ Command succeeded, no changes needed. |
T-Gro
commented
Feb 10, 2026
| @@ -0,0 +1,140 @@ | |||
| # Regression: FS0229 B-Stream Misalignment in TypedTreePickle | |||
Member
Author
There was a problem hiding this comment.
Keep this .md for interested reviewers, but I will delete it before merging.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Adds IcedTasks, FsToolkit.ErrorHandling, and OpenTK to the PR regression testing pipeline, and fixes two compiler regressions discovered by running these libraries against the current compiler.
Regression Test Infrastructure
New libraries added to
azure-pipelines-PR.ymlb90f2b389cd957e35168e1d2Template improvements (
eng/templates/regression-test-jobs.yml)buildScriptentries can now contain multiple commands separated by;;, executed sequentially (first failure stops the job). This allows testing multiple build targets in a single matrix entry without duplicating jobs..are converted to_for valid AzDO identifiers.net9.0.dotnet tool restorepre-step: Prevents race conditions when repos use local tools (e.g., FAKE).Compiler Bug Fix: FS3356 — False positive for instance extension members
File:
src/Compiler/Checking/PostInferenceChecks.fsCheckForDuplicateExtensionMemberNames(added by #18821) was flagging all extension members on types with the same simple name from different namespaces. However, the underlying IL collision problem only affects static extension members — instance extension members compile with the extended type as the first IL parameter, which differentiates their signatures.Fix: Added
&& not (v.IsInstanceMember)filter so only static extension members are checked.Affected library: IcedTasks — defines instance extension members on
AsyncValueTaskMethodBuilderandAsyncValueTaskMethodBuilder<'T>(same simple name, different generic arity).Tests: Updated
DuplicateExtensionMemberTests.fsto use static members (the actual IL collision case) and added companion tests showing instance members are allowed. Added additional regression tests inExtensionMethodTests.fs.Compiler Bug Fix: FS0229 — B-stream misalignment in metadata pickling
File:
src/Compiler/TypedTree/TypedTreePickle.fsDocumentation:
docs/regression-fs0229-bstream-misalignment.mdF# compiler metadata uses two parallel streams: stream A (main tags/data) and stream B (nullness info + newer constraint data like
NotSupportsNull). WhenlangFeatureNullnessis disabled (LangVersion < 9.0):p_ty2): Conditionally skipped nullness B-bytes for type tags 1–4u_ty): Unconditionally read B-bytes for those same tagsp_tyar_constraints): Unconditionally wrote constraint data (e.g.,NotSupportsNullfrom BCL types) to B-streamThis caused B-stream misalignment — the reader consumed constraint bytes as nullness values, eventually hitting an invalid tag and producing
FS0229: u_ty - 4/B.Fix: Added
else p_byteB 0 stto all four type cases inp_ty2, ensuring a B-byte is always written regardless oflangFeatureNullness. Value0means "AmbivalentToNull" and is already handled by all reader cases.Affected library: FsToolkit.ErrorHandling — uses
<LangVersion>8.0</LangVersion>fornetstandard2.0/netstandard2.1TFMs, which disableslangFeatureNullness.Timeline: Bug was latent since nullness checking (#15181, Jul 2024) but only manifested after
AllowsRefStruct(#17706, Sep 2024) added unconditional B-stream writes for constraints.Tests: Two regression tests in
ImportTests.fs— compile a library at LangVersion 8.0 with BCL-constrained generics, then reference it and verify no FS0229.Other changes
ImportTests.fsadded to.fsproj: This file was migrated by Goodbye Perl #19226 but never included inFSharp.Compiler.ComponentTests.fsproj, so all its tests were dormant. Fixed one pre-existing warning 52 issue on net472.