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

Explicitly depend on racc to fix specs for Ruby 3.3+ #123

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mattbrictson
Copy link

@mattbrictson mattbrictson commented Nov 20, 2023

On Ruby 3.3.0-preview3, knapsack's own specs fail:

$ ruby -v
ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-darwin23]

$ bundle exec rspec spec
An error occurred while loading spec_helper. - Did you mean?
                    rspec ./spec/spec_helper.rb

Failure/Error: require 'spinach'

LoadError:
  cannot load such file -- racc/parser.rb

This is because in Ruby 3.3, the racc gem is no longer a default gem. This change is mentioned in the Ruby 3.3.0-preview2 release notes:

The following default gem is now bundled.
racc 1.7.1

Since knapsack's specs rely on the racc gem being present, this means that in Ruby 3.3 racc must be explicitly declared as a development dependency in the Gemfile.

This commit adds the racc dependency, conditionally based on the Ruby version. With this change, the specs now pass.

$ ruby -v
ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-darwin23]

$ bundle exec rspec spec
...
Finished in 1.91 seconds (files took 0.42586 seconds to load)
210 examples, 0 failures

On Ruby 3.3.0-preview3, knapsack's own specs fail:

```
$ ruby -v
ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-darwin23]

$ bundle exec rspec spec
An error occurred while loading spec_helper. - Did you mean?
                    rspec ./spec/spec_helper.rb

Failure/Error: require 'spinach'

LoadError:
  cannot load such file -- racc/parser.rb
```

This is because in Ruby 3.3, the racc gem is no longer a default gem.
This change is mentioned in the Ruby 3.3.0-preview2 release notes:

> The following default gem is now bundled.
> racc 1.7.1

https://www.ruby-lang.org/en/news/2023/09/14/ruby-3-3-0-preview2-released/

Since knapsack's specs rely on the racc gem being present, this means
that in Ruby 3.3 racc must be explicitly declared as a development
dependency in the Gemfile.

This commit adds the racc dependency, conditionally based on the Ruby
version. With this change, the specs now pass.

```
$ ruby -v
ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-darwin23]

$ bundle exec rspec spec
...
Finished in 1.91 seconds (files took 0.42586 seconds to load)
210 examples, 0 failures
```
@ArturT
Copy link
Member

ArturT commented Nov 20, 2023

Thanks for reporting this and proposing a fix.

Why not always require racc in gemspec, no matter the Ruby version? Would this break anything for older Ruby versions?

# knapsack.gemspec
spec.add_development_dependency 'racc', '>= 1.7.3'

I added this PR to our internal backlog to be prioritized.

@ArturT ArturT added the bug label Nov 20, 2023
@mattbrictson
Copy link
Author

I was worried that the racc dependency would fail under the old versions in the CI matrix (2.3). I'll move it to the gemspec to see what happens.

@mattbrictson
Copy link
Author

I tested using the ruby:2.3 Docker image and it looks like the gemspec approach works. We'll see for sure when the CI workflow is run.

@ArturT
Copy link
Member

ArturT commented Nov 20, 2023

I approved the GitHub Actions workflow for your PR.

@mattbrictson
Copy link
Author

@ArturT thanks! Tests are passing for all Ruby versions in CI.

@ArturT
Copy link
Member

ArturT commented Nov 20, 2023

I'm trying to understand how urgent this PR is. Is the issue limited only to running knapsack tests locally with Ruby 3.3+?

Or have you experienced issues running your project's tests on your CI using Ruby 3.3+ and knapsack 4.0.0?

@mattbrictson
Copy link
Author

This is not urgent at all. It is only applicable to running knapsack tests locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants