Fix integration test flakes in CI #4391
Merged
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
`helix_view::apply_transaction` closes over `Document::apply` and `View::apply` to ensure that jumplist entries are updated when a document changes from a transaction. `Document::apply` shouldn't be called directly - this helper function should be used instead.
This change removes language server configuration from the default languages.toml config for integration tests. No integration-tests currently depend on the availability of a language server but if any future test needs to, it may provide a language server configuration by passing an override into the `test_syntax_conf` helper. Language-servers in integration tests cause false-positive failures when running integration tests in GitHub Actions CI. The Windows runner appears to have `clangd` installed and all OS runners have the `R` binary installed but not the `R` language server package. If a test file created by `tempfile::NamedTempFile` happens to have a file extension of `r`, the test will most likely fail because the R language server will fail to start and will become a broken pipe, meaning that it will fail to shutdown within the timeout, causing a false-positive failure. This happens surprisingly often in practice. Language servers (especially rust-analyzer) also emit unnecessary log output when initializing, which this change silences.
This function is not currently used but is likely to be useful in the future, so this change silences the dead_code warning.
|
Thanks! |
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.
Write path fixes (#2267) introduced some flaky behavior in the integration test suite that happens when running within GitHub Actions.
The Windows runner has
clangdinstalled, but it sometimes takes too long to shutdown which causes false-positive test failures. All runners have theRbinary installed but not the language server package. This causes a flake whenevertempfile::NamedTempFilecreates a file which is mistaken as R (this happens surprisingly often in practice). The R binary starts but immediately crashes because the language server package is not installed. Then the language server cannot be shutdown in the expected timeframe, resulting in a false-positive failure.Here I've removed all language server configuration for the integration tests by default with the ability to add back the configuration explicitly. This fixes the flakes as far as I've tested and silences some unnecessary output when running locally (from
rust-analyzer).I've also added some small fixes to...
helix_view::apply_transactioninstead ofDocument::apply(see Apply transactions to jumplist selections #4186)AppBuilder::with_config