-
-
Notifications
You must be signed in to change notification settings - Fork 108
fix: CI infrastructure issues #4502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Fix flaky WaitsFor_Performance_Many_Quick_Polls test on net472
- The 1ms polling interval was unrealistic on .NET Framework where
the minimum timer resolution is ~15ms
- Changed to 10ms interval and reduced target count from 100 to 20
- Add net10.0 target framework to FSharp and VB NuGet tester projects
- Pipeline was running with --framework net10.0 but projects only
supported up to net9.0
SummaryFixes two CI infrastructure issues: a flaky performance test on .NET Framework and missing net10.0 target in F#/VB NuGet tester projects. Critical IssuesNone found ✅ SuggestionsTest Coverage ConsiderationThe Alternative approaches to consider:
However, the current fix is reasonable and makes the test reliable across all platforms, which is the primary goal. Verdict✅ APPROVE - No critical issues The changes are well-explained, address real CI failures, and align the F#/VB projects with the C# project's target frameworks. The test adjustment is a pragmatic fix for .NET Framework's timer resolution limitations. |
SummaryFixes two CI infrastructure issues: flaky timer test on .NET Framework and missing net10.0 targets in F#/VB NuGet test projects. Critical IssuesNone found ✅ SuggestionsDocument the attribute removalThe PR description mentions two fixes but doesn't explain the removal of While the removal appears correct (the
This helps future readers understand all changes in the PR. Previous Review StatusUnable to retrieve previous comments due to GitHub API scope limitations. Verdict✅ APPROVE - No critical issues. The changes are correct and well-reasoned:
Optional: Consider updating the PR description for completeness. |
Summary
Fixes multiple CI failures that have been affecting the main branch.
Issues Fixed
1. Flaky
WaitsFor_Performance_Many_Quick_Pollstest (Windows, net472)The test was using a 1ms polling interval, which is unrealistic on .NET Framework 4.7.2 where the minimum Windows timer resolution is typically 15-16ms. The test expected 100+ polling iterations in 5 seconds but only achieved ~71 before timing out.
Fix: Changed to 10ms polling interval and reduced target count from 100 to 20, which still validates the "many quick polls" behavior while being reliable across all platforms.
2. Missing net10.0 target in FSharp/VB NuGet tester projects
The
TestFSharpNugetPackageModuleandTestVBNugetPackageModulewere failing because the pipeline runs with--framework net10.0but these projects only had target frameworks up to net9.0.Fix: Added
net10.0to theTargetFrameworksin both projects to match the C# NuGet tester.Test plan