-
Notifications
You must be signed in to change notification settings - Fork 154
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
Fix CI Tests #955
Merged
Merged
Fix CI Tests #955
Conversation
This file contains 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
`ReplaceWithSuggestion.can change namespace in open` fails sometimes on CI: Diag 39 ("... is not defined") does exist -- but doesn't always contains suggestions ("Maybe you want one of the following:") Not reproducible locally -- and on CI not always the case
-> can be run alone (vs. prev: requires all tests to be run)
Sometimes a `RenameParamToMatchSignature`Test fails on CI Seems to be a caching issue: Wants to replace incorrect variable name with other incorrect variable name. Diagnostics seems to be old too -> Works with old data -> Use unique names in tests to make tests cases more distinct and recognizable. **Example**: Test "FSAC.lsp.Ionide WorkspaceLoader.CodeFix tests.RenameParamToMatchSignature.can rename parameter with ' (not in last place) in signature in F# function" fails with "No matching CodeAction". In Signature: `v'2`, in implementation: `value` But CodeFix for: Replace `v` with ` ``my value`` ` -> That are the names of another test Not sure if error in Diag Caching in FSAC or issue in retrieving correct diags from Diags stream. Most likely the latter
Issue with new version: `FSAC.lsp.Ionide WorkspaceLoader.CodeFix tests.ReplaceWithSuggestion.can change namespace in open` always fails: F# compiler diagnostic message never contains any name suggestions Diagnostic message should look like: ``` The namespace 'RegularEcpressions' is not defined. Maybe you want one of the following: RegularExpressions ``` But on ubuntu 22 there's no suggestion, only ``` The namespace 'RegularEcpressions' is not defined. ```
Issue on CI with short wait times for late diags -> Often resulted in test failures Different delays for running test locally and on CI * locally: 7ms * on CI: 25ms * Env Var: [`CI`](https://docs.github.com/en/actions/learn-github-actions/environment-variables#default-environment-variables) * Additional: Environment Variable `FSAC_WaitForLateDiagnosticsDelay` to set custom delay (in ms) * Fails when no valid int number Note: With `dotnet test` and invalid Env Var, the error message doesn't mention invalid Env Var, but instead just: `The type initializer for '<StartupCode$FsAutoComplete-Tests-Lsp>.$Utils.Server' threw an exception.` Error Message with Env Var is in InnerException which isn't displayed with `dotnet test`. It is with `dotnet run --project ./test/FsAutoComplete.Tests.Lsp` (incl. its StackTrace) Note: Not ideal solution: might still fail. After all: is just some waiting and hoping for every diag to arrive in time. Correct handling requires larger rewrite (maybe sending file versions with diags?) But should fail way less then with shorter delay
Nice, seeing all green is very gratifying :) About the update to 6.0.3xx - I've done it incidentally in #954, but I think something has changed such that the tests are deadlocking - my best guess is that there's a new diagnostic or something that's invalidating one of the few remaining unbounded Waits we have in the test harness. |
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.
Fix Windows Tests ... by using newer windows version (win server 2022 vs 2019)
Don't know exactly why -- but newer os seems to have better lock handling.
This issue (very rarely) occurs on other oss too
-> still good idea to update to
dotnet 6.0.300
which should fix this (as noted in #943 (comment)) (but not done here)Fixish occasional test failures
Reason: When Document waits for latest diagnostics, it has to wait a short time to get all diagnostics.
This wait time was sometimes to short on CI -> got incorrect diags -> test fails
I now wait a bit longer on CI tests (25ms vs 7ms for local test runs)
Tests now seem to succeed -- at least most of the time. Sometimes they still fail.
-> This isn't a real fix -- just hoping delay is now long enough to get all diags...
To really fix this we must ensure we get all correct diags. But this requires some rewriting of Diagnostics handling in FSAC
Besides updating windows I updated macos too -- but not ubuntu (currently used:
20.04
, latest:22.4
).Reason: with ubuntu 22, test
FSAC.lsp.Ionide WorkspaceLoader.CodeFix tests.ReplaceWithSuggestion.can change namespace in open
always fails:Test is about replacing
open System.Text.RegularEcpressions
withRegularExpressions
.The CodeFix extracts that suggestion from F# compiler diagnostic message, which should look like:
But on ubuntu 22 there's no suggestion, just:
-> F# compiler produces correct Diagnostic (Code 39), but the message contains no suggestions.
All other suggestion tests (with suggestions in other locations like misspelled variable name) work
I have not the slightest idea why
(I cannot reproduce the issue locally: on local ubuntu 22 (docker) the test works just fine)