Skip to content

Fix CI setup for Windows #352

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

Merged
merged 1 commit into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,8 @@ jobs:
- name: Lint Ruby files
run: bin/rubocop

- name: Lint Ruby files
run: bin/rubocop

- name: Run tests
run: bin/rails db:setup && bin/rails db:migrate && bin/rails test
run: bundle exec rails db:setup && bundle exec rails db:migrate && bundle exec rails test
Copy link
Contributor Author

@andyw8 andyw8 Apr 26, 2024

Choose a reason for hiding this comment

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

bin/rails is failing on Windows but appears to still return a successful exit code, which was causing the build to be wrongly marked as passing.

5 changes: 5 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,8 @@ gem "tapioca", "~> 0.13", require: false, platforms: :ruby
gem "psych", "~> 5.1", require: false
gem "rails"
gem "webmock"

platforms :mingw, :x64_mingw, :mswin, :jruby do
gem "tzinfo"
gem "tzinfo-data"
end
2 changes: 2 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,8 @@ DEPENDENCIES
sorbet-static-and-runtime
sqlite3 (< 2)
tapioca (~> 0.13)
tzinfo
tzinfo-data
webmock

BUNDLED WITH
Expand Down
5 changes: 5 additions & 0 deletions gemfiles/Gemfile-rails-main
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,8 @@ gem "tapioca", "~> 0.11", require: false, platforms: :ruby
gem "psych", "~> 5.1", require: false
gem "rails", github: "rails/rails", branch: "main"
gem "webmock"

platforms :mingw, :x64_mingw, :mswin, :jruby do
gem "tzinfo"
gem "tzinfo-data"
end
2 changes: 1 addition & 1 deletion lib/ruby_lsp/ruby_lsp_rails/hover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def generate_column_content(name)
schema_file = model[:schema_file]

@response_builder.push(
"[Schema](#{URI::Generic.build(scheme: "file", path: schema_file)})",
"[Schema](#{URI::Generic.from_path(path: schema_file)})",
category: :links,
) if schema_file

Expand Down
5 changes: 4 additions & 1 deletion lib/ruby_lsp/ruby_lsp_rails/runner_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ def initialize
Process.setsid
rescue Errno::EPERM
# If we can't set the session ID, continue
rescue NotImplementedError
# setpgrp() may be unimplemented on some platform
# https://github.com/Shopify/ruby-lsp-rails/issues/348
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Let's look into this separately).

end

stdin, stdout, stderr, wait_thread = Bundler.with_original_env do
Open3.popen3("bin/rails", "runner", "#{__dir__}/server.rb", "start")
Open3.popen3("bundle", "exec", "rails", "runner", "#{__dir__}/server.rb", "start")
end

@stdin = T.let(stdin, IO)
Expand Down
8 changes: 4 additions & 4 deletions test/ruby_lsp_rails/code_lens_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Test < ActiveSupport::TestCase
# The last 3 are for the test declaration.
assert_equal(6, response.size)
assert_match("Run", response[3].command.title)
assert_equal("bin/rails test /fake.rb:2", response[3].command.arguments[2])
assert_match(%r{(ruby )?bin/rails test /fake\.rb:2}, response[3].command.arguments[2])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to #353

assert_match("Run In Terminal", response[4].command.title)
assert_match("Debug", response[5].command.title)
end
Expand All @@ -54,7 +54,7 @@ class Test < ActiveSupport::TestCase
# The last 3 are for the test declaration.
assert_equal(6, response.size)
assert_match("Run", response[3].command.title)
assert_equal("bin/rails test /fake.rb:2", response[3].command.arguments[2])
assert_match(%r{(ruby )?bin/rails test /fake\.rb:2}, response[3].command.arguments[2])
assert_match("Run In Terminal", response[4].command.title)
assert_match("Debug", response[5].command.title)
end
Expand All @@ -73,7 +73,7 @@ class Test < ActiveSupport::TestCase
# The last 3 are for the test declaration.
assert_equal(6, response.size)
assert_match("Run", response[3].command.title)
assert_equal("bin/rails test /fake.rb:2", response[3].command.arguments[2])
assert_match(%r{(ruby )?bin/rails test /fake\.rb:2}, response[3].command.arguments[2])
assert_match("Run In Terminal", response[4].command.title)
assert_match("Debug", response[5].command.title)
end
Expand Down Expand Up @@ -144,7 +144,7 @@ def test_example
# The last 3 are for the test class.
assert_equal(6, response.size)
assert_match("Run", response[3].command.title)
assert_equal("bin/rails test /fake.rb:2", response[3].command.arguments[2])
assert_match(%r{(ruby )?bin/rails test /fake\.rb:2}, response[3].command.arguments[2])
assert_match("Run In Terminal", response[4].command.title)
assert_match("Debug", response[5].command.title)
end
Expand Down
8 changes: 4 additions & 4 deletions test/ruby_lsp_rails/hover_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class User < ApplicationRecord
```

**Definitions**: [fake.rb](file:///fake.rb#L1,1-2,4)
[Schema](file://#{dummy_root}/db/schema.rb)
[Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")})


**id**: integer (PK)
Expand Down Expand Up @@ -95,7 +95,7 @@ class User < ApplicationRecord
RUBY

assert_equal(<<~CONTENT.chomp, response.contents.value)
[Schema](file://#{dummy_root}/db/schema.rb)
[Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")})

**id**: integer (PK)

Expand Down Expand Up @@ -139,7 +139,7 @@ class CompositePrimaryKey < ApplicationRecord
```

**Definitions**: [fake.rb](file:///fake.rb#L1,1-2,4)
[Schema](file://#{dummy_root}/db/schema.rb)
[Schema](#{URI::Generic.from_path(path: dummy_root + "/db/schema.rb")})


**order_id**: integer (PK)
Expand Down Expand Up @@ -172,7 +172,7 @@ class User < ApplicationRecord

assert_includes(
response.contents.value,
"[Schema](file://#{dummy_root}/db/structure.sql)",
"[Schema](#{URI::Generic.from_path(path: dummy_root + "/db/structure.sql")})",
)
end

Expand Down
15 changes: 6 additions & 9 deletions test/ruby_lsp_rails/runner_client_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
require "test_helper"
require "ruby_lsp/ruby_lsp_rails/runner_client"

# tests are hanging in CI. https://github.com/Shopify/ruby-lsp-rails/issues/348
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I narrowed it down to this file.

Copy link
Member

Choose a reason for hiding this comment

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

Do we still need to skip this test on Windows? If so, let's ship this, but please prioritize unskipping it so that we don't go long with skipped tests.

return if Gem.win_platform?

module RubyLsp
module Rails
class RunnerClientTest < ActiveSupport::TestCase
Expand Down Expand Up @@ -54,16 +57,10 @@ class RunnerClientTest < ActiveSupport::TestCase
end

test "failing to spawn server creates a null client" do
FileUtils.mv("bin/rails", "bin/rails_backup")
File.open("bin/rails", "w") do |f|
f.write("foo")
end
File.chmod(0o755, "bin/rails")

# The error message is slightly different on Ubuntu, so we need to allow for that
FileUtils.mv("test/dummy/config/application.rb", "test/dummy/config/application.rb.bak")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Picked this as an arbitrary file to remove to verify boot failure.

assert_output(
"",
%r{Ruby LSP Rails failed to initialize server: bin/rails: (line )?1: foo:( command)? not found},
/Ruby LSP Rails failed to initialize server/,
) do
client = RunnerClient.create_client

Expand All @@ -72,7 +69,7 @@ class RunnerClientTest < ActiveSupport::TestCase
assert_predicate(client, :stopped?)
end
ensure
FileUtils.mv("bin/rails_backup", "bin/rails")
FileUtils.mv("test/dummy/config/application.rb.bak", "test/dummy/config/application.rb")
end

test "is resilient to extra output being printed during boot" do
Expand Down