-
-
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
Dumping Hash with default_proc that was reset #4302
Comments
I guess this is related to this failure: |
@guilleiguaran Very likely, as well as some failures @enebo was working on in ActiveRecord! |
@enebo please let me know if you already worked on it. If not, I'll be happy to submit a PR |
@guilleiguaran OMGZ you rock. I have been trying to figure out what was supplying a default Hash to marshal in a local unit test in arjdbc and did not realize 2.3 allowed this. So it is in fact a JRuby bug and not a arjdbc one. This will save me time since I will now look in the right place! |
@guilleiguaran wow github is not working great this morning. Yeah if you can figure this out then go for it. I would figure out the commit in MRI and work from there. |
So I cloned the repo, installed dependencies and tried to run test related to the Hash class:
But many of them failed:
It is supposed to be stable and green atm? Or is there some specific branch I should look at? |
We exclude some tests. Some other sensible like tests involving callcc (which we cannot support) and others we look to people like @kirs to help us :) |
@kirs sorry I should have pointed out we use 'minitest-excludes (2.0.0)' to exclude tests we do not pass so we can be green but know what else we can potentially improve. |
Thanks! |
@kirs update those docs to point out our use of minitest-excludes. |
The proper way to run with excludes is described in BUILDING.md a few lines below the initial runner line: https://github.com/jruby/jruby/blame/master/BUILDING.md#L140 |
My comment was confusing but I did change the line above that. It sounded like I wanted him to change it but I did...hmm maybe I should revert that |
When we set `default_proc` to nil, we also have to update the internal flag. Fixes jruby#4302
Made my first PR to JRuby 🎉 |
Running on
jruby 9.1.6.0 (2.3.1) 2016-11-09 0150a76 Java HotSpot(TM) 64-Bit Server VM 25.45-b02 on 1.8.0_45-b14 +jit [darwin-x86_64]
Expected Behavior
JRuby should be able to dump a hash that was initialized with
default_proc
that was later set tonil
.Actual Behavior
JRuby currently ignores
default_proc
set tonil
and gives an error that a hash with default_proc cannot be dumped.@headius @guilleiguaran
The text was updated successfully, but these errors were encountered: