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

Improve client integration tests #2033

Merged
merged 2 commits into from
May 29, 2024
Merged

Conversation

vinistock
Copy link
Member

@vinistock vinistock commented May 10, 2024

Motivation

While working on #2017, I realized that our client integration test was not activating Ruby (it just used the one available on CI), which was actually masking some issues.

This PR improves our tests and fixes the related issues to make tests pass.

Implementation

The PR is split by commit:

  1. Environment variables are case sensitive and Windows uses the name Path instead of PATH, which was causing us to miss the general environment on the RubyInstaller integration
  2. This commit includes the improvements to the test, which now displays more information about failures and ensures the right Ruby is activated to run integrations

Automated Tests

Added tests.

@vinistock vinistock added bugfix This PR will fix an existing bug vscode This pull request should be included in the VS Code extension's release notes labels May 10, 2024
@vinistock vinistock self-assigned this May 10, 2024
@vinistock vinistock requested a review from a team as a code owner May 10, 2024 18:11
@vinistock vinistock requested review from andyw8 and st0012 May 10, 2024 18:11
lib/ruby_lsp/listeners/document_link.rb Outdated Show resolved Hide resolved
@@ -11,6 +11,9 @@ def from_path(path:, fragment: nil, scheme: "file")
# On Windows, if the path begins with the disk name, we need to add a leading slash to make it a valid URI
escaped_path = if /^[A-Z]:/i.match?(path)
DEFAULT_PARSER.escape("/#{path}")
elsif path.start_with?("//?/")
# Some paths on Windows start with "//?/". This is a special prefix that allows for long file paths
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤯

@vinistock vinistock force-pushed the vs/improve_client_integration_tests branch from 35f66a8 to c979ae7 Compare May 29, 2024 19:01
@vinistock vinistock enabled auto-merge (squash) May 29, 2024 19:11
@vinistock vinistock merged commit d993c5e into main May 29, 2024
34 checks passed
@vinistock vinistock deleted the vs/improve_client_integration_tests branch May 29, 2024 19:12
andyw8 pushed a commit that referenced this pull request May 29, 2024
* Ensure RubyInstaller is retrieving the right PATH

* Improve integration tests to activate the current Ruby
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug vscode This pull request should be included in the VS Code extension's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants