-
-
Notifications
You must be signed in to change notification settings - Fork 235
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
What would be needed for RSpec to fully support TruffleRuby? #68
Comments
There is not enough bandwidth amongst the team to add another Ruby to the supported set. Unlike Rails we are a small team without the backing of any companies. So the first step would be someone volunteering to maintain TruffleRuby support within RSpec. I would then consider allowing truffle ruby to be added to the general CI matrix for a stable release (no head, I don't want more instability in our builds) when truffle passes stabley. I'll happily setup a long running "truffle-support-dev" branch to facilitate this if the person who steps up to maintain truffle support wishes it. |
@bjfish if you need any help here pls let me know. Currently I am looking at rspec-core project and already filled one issue which may be related to the failing specs of rspec-core: oracle/truffleruby#2602 |
(former rspec maintainer chiming in) take this very much as the peanut gallery but I wonder if truffleruby should CI against RSpec, so that you know ahead of time if something is going to break |
Very good point, @penelopezone! We don't have as many changes as TruffleRuby, and breakages to RSpec+TruffleRuby compatibility will supposedly mostly come from TruffleRuby changes, not changes in RSpec. |
Currently TruffleRuby tests a small number of gems in its CI, i.e. in the gate/pre-merge CI (e.g. There is existing infrastructure to test a large amount of gems, asynchronously, this is how TruffleRuby plans to test RSpec and many other gems. Having TruffleRuby in RSpec's CI (even if daily/weekly) is a separate test: it means latest RSpec + fixed TruffleRuby, while the above is testing latest TruffleRuby + fixed RSpec, and so that's useful regardless. |
Sounds reasonable. It must be possible to run it on merge to |
I started working on Truffleruby support for Rspec-core here rspec/rspec-core#2942 can some one review the PR? |
I ran the test suite locally and saw 33 failures for rspec-core
rspec-expectations
** rspec-mocks**
rspec-rails
|
I got a fix for the kwargs-related failures: rspec/rspec-mocks#1464 |
I've submitted a possible fix for the |
To clarify my comments from the original rspec-core pr, #2942, I'd actually be ok with having a seperate CI build on our repos that runs cron and on branches with truffle in the name. My hesistation about adding truffle is to the main CI pass that runs on merges to main and to PR reviews. A seperate workflow that doesn't affect MRI prs would be fine with me. |
Right, I think adding a daily or weekly run would be helpful, once truffleruby passes all specs of one of the rspec gems (gem by gem is helpful for incremental progress, especially given the current multi-repo setup). |
Yes. |
Most of the issues in rspec-core were resolved here rspec/rspec-core#2946 , except for one:
|
I looked into the above, and that difference is expected. For: begin
exit 999
rescue SystemExit => e
puts e.backtrace
end
I'll make a PR to adapt the expected output. For CRuby it shows For some reason running rspec-core specs with |
Created a PR for a failing expectations spec: |
I think we should disable the usage of Ripper by RSpec for TruffleRuby, in practice it seems currently very slow (seconds of pause just before showing errors & backtraces) and so it hurts the user experience a lot more than help. @bjfish Could you look into that? |
Just for my understanding, what Ripper was used for in RSpec? |
I read that spec but it didn't help me much 😅 |
We had some discussions about this years ago but I think it's time to clarify the requirements.
Of course, TruffleRuby would need pass the RSpec test suite, I think it's quite close to that if not already the case.
Maybe @gogainda or @bjfish can check that by running the CI on forks, adding truffleruby to the matrix, and report the results here.
Anything else?
When TruffleRuby already runs Rails and is already in many other gems CI, it does feel weird that such a common gem as RSpec doesn't officially support or test TruffleRuby in CI.
OTOH AFAIK RSpec already works fine on TruffleRuby in practice for running specs, I don't remember any issue running RSpec on TruffleRuby in many years.
If the concern is adding TruffleRuby in CI would slow down development due to e.g. longer CI times for PRs, I think an easy way to address that would be to make the CI job a daily or weekly one.
Still, I think we should measure if it adds any time for a PR workflow and how much, per rspec repo (e.g., maybe rspec-rails takes a while and rspec-support takes very little time).
Another concern I remember is wanting a release of truffleruby in CI, and not truffleruby-head in CI.
That's fine, it might just take a bit longer to wait for the next release.
The text was updated successfully, but these errors were encountered: