Skip to content
This repository was archived by the owner on May 30, 2024. It is now read-only.

Move environment manipulation completely to the Ruby object#440

Merged
vinistock merged 1 commit intomainfrom
vs/move_environment_to_ruby
Mar 15, 2023
Merged

Move environment manipulation completely to the Ruby object#440
vinistock merged 1 commit intomainfrom
vs/move_environment_to_ruby

Conversation

@vinistock
Copy link
Copy Markdown
Member

The issue in #429 got me thinking about how we also mutate the environment during activation and if we manipulating it in the client made sense.

I think all environment manipulation should be contained in the Ruby class - since we're interested in the Ruby environment after all. This PR

  • moves all env related behaviour inside Ruby
  • stops mutating process.env completely
  • adds a few tests, limited to what we can verify without a version manager installed
  • adds setup-ruby to CI, since we need some Ruby to be installed in order to check for the information

@vinistock vinistock requested a review from a team as a code owner March 10, 2023 20:34
@vinistock vinistock self-assigned this Mar 10, 2023
@st0012
Copy link
Copy Markdown
Member

st0012 commented Mar 13, 2023

I try to run the tests locally but it failed like 80% of the time.

Loading development extension at /Users/hung-wulo/src/github.com/Shopify/vscode-ruby-lsp
Unexpected token � in JSON at position 0

  Telemetry
    ✔ Events are sent via the defined API
    ✔ The API object is acquired via command
  Ruby environment activation
    1) Activate fetches Ruby information when outside of Ruby LSP
    ✔ Activate fetches Ruby information when working on the Ruby LSP
  3 passing (2s)
  1 failing
  1) Ruby environment activation
       Activate fetches Ruby information when outside of Ruby LSP:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/hung-wulo/src/github.com/Shopify/vscode-ruby-lsp/out/test/suite/ruby.test.js)
        at listOnTimeout (node:internal/timers:559:17)
        at process.processTimers (node:internal/timers:502:7)

Error: 1 tests failed.
        at /Users/hung-wulo/src/github.com/Shopify/vscode-ruby-lsp/out/test/suite/index.js:49:32
        at done (/Users/hung-wulo/src/github.com/Shopify/vscode-ruby-lsp/node_modules/mocha/lib/mocha.js:1012:7)
[main 2023-03-13T06:59:06.000Z] [UtilityProcess id: 1, type: extensionHost, pid: 55407]: waiting to exit...
[main 2023-03-13T06:59:06.011Z] [UtilityProcess id: 1, type: extensionHost, pid: 55407]: received exit event with code 0
[main 2023-03-13T06:59:06.011Z] Extension host with pid 55407 exited with code: 0, signal: unknown.
Exit code:   1
Failed to run tests
error Command failed with exit code 1.

Possibly related screenshot:

Screenshot 2023-03-13 at 14 59 05

@vinistock vinistock force-pushed the vs/move_environment_to_ruby branch from 089c1b9 to e0483b7 Compare March 13, 2023 13:51
@vinistock
Copy link
Copy Markdown
Member Author

I think it's because I used afterEach instead of after for restoring your configured version manager. Can you give it another try?

Comment thread src/test/suite/ruby.test.ts Outdated
@st0012
Copy link
Copy Markdown
Member

st0012 commented Mar 14, 2023

@vinistock Tests now all pass locally. Thanks 👍

@vinistock vinistock force-pushed the vs/move_environment_to_ruby branch from e0483b7 to 318e5ce Compare March 14, 2023 15:29
@vinistock vinistock merged commit 8af52dd into main Mar 15, 2023
@vinistock vinistock deleted the vs/move_environment_to_ruby branch March 15, 2023 13:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants