-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
spec/ruby: Bring in some changes from upstream #5140
Conversation
yep as you figured specs eventually end up here as they get merged, just check history : |
Sorry about some confusion on master about failing tests. I made a change for 9.1 that caused a number of suites to start terminating early with a successful return code, so it looked like things were suddenly green. We will follow normal process for updating specs, so we get the appropriate logs. @perlun If you like you could rebase off current master and force push, so we'd get an accounting of new failures. |
This brings in the changes I did in ruby/spec#596 into our tree also. They highlight a current issue in JRuby that should be fixed at one point or another. (There were other, mostly BigDecimal-related changes also, but I picked _only_ the relevant changes from the spec tree. I don't know what the official process is for updating these; I looked for a Rake task but couldn't find any.)
64b49ba
to
6bdfd05
Compare
@headius Thank you for that. Rebased & force-pushed, we should be start seeing the failures in a short while. |
Interestingly enough, I can't see the expected test failures now (but it fails on a random bunch of other tests. 😉) I'll try to update my |
To summarize, for specs fixing bugs in JRuby, the recommended process is adding the spec directly in JRuby's spec/ruby, which is synchronized ~once a month with ruby/spec and other copies of the specs. If there is any question regarding new specs, feel free to ping me. @enebo @headius Maybe I should add a |
Alright. In this case, some of it was fixed in JRuby but some of it is still unfixed. I was under the impression that the CI runs of this PR should highlight the latter, but I couldn't find it in the output (there are unfortunately a few other CI errors at the moment.) |
I tried rebuilding my local JRuby and can verify that the specs still fail with the latest
|
Ping - are there any major problems with merging this? |
Looks fine for me, since this is the exact same diff as in ruby/spec it should be easy to ignore it when doing the ruby/spec update (in a few days, I do it at the end of the month). I'll let JRuby devs merge this if they want though. |
I updated all specs in #5154. I recommend to add specs directly in JRuby when working on a bug, so then the specs can be immediately run in JRuby's CI. The specs are anyway integrated in ruby/spec ~once a month, so other implementations can also benefit from them. |
This is a followup to #5134 which brings in the changes I did in ruby/spec#596 into our tree also. They highlight a current issue in JRuby that should be fixed at one point or another.
(There were other, mostly
BigDecimal
-related changes coming from there also, but I picked only the relevant changes from the spec tree for now. I don't know what the official process is for updating these; I looked for a Rake task but couldn't find any.)Here are the results from these tests, illustrating the problem & current, incorrect behavior in JRuby. These specs pass on MRI.