Skip to content
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 8 commits into from
Jul 1, 2022
Merged

Fix CI Tests #955

merged 8 commits into from
Jul 1, 2022

Conversation

Booksbaum
Copy link
Contributor

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 with RegularExpressions.
The CodeFix extracts that suggestion from F# compiler diagnostic message, which 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, just:

The namespace 'RegularEcpressions' is not defined.

-> 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)

`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
@baronfel
Copy link
Contributor

baronfel commented Jul 1, 2022

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.

@baronfel baronfel merged commit 9f58993 into ionide:main Jul 1, 2022
@Booksbaum Booksbaum deleted the windows branch July 5, 2022 19:27
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