-
-
Notifications
You must be signed in to change notification settings - Fork 253
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
Oj.dump throws RangeError for Time value (Windows only?) #485
Comments
I'm not setup for windows. Would you be able to test changes for me? |
Potentially yes, although I do see that you have Windows CI set up so hopefully that will help as well |
I checked in a |
The new test is failing on the branch:
However, I actually have test failures on master as well- all time stuff. So clearly something is weird in my environment that is affecting this.
|
It looks like a timezone issue creeping in that did not show up on appveyor. I'll look into it and see if I can add some debug info on the branch. I'd like to figure out how you environment is different from appveyor as well. |
I turned on trace for the custom test. Will focus on that one until I figure out what is going on. I'm going to make a guess that you are in either a +8 or -8 timezone. Is that correct? |
No, I'm US EDT (-4) Time.now
=> 2018-07-18 10:36:36 -0400 |
It has been a busy week. Anyway, I checked in a version with some extra print statements to narrow down where in the custom test the failure is occurring. There are 4 possible test cases. Please let me know what the results are. This is the first step of several I'm afraid since I can't reproduce it here. |
Where you able to run the tests? |
I had some time to revisit this. Still seeing test failures, but also seeing your diagnostic output. Here's the latest compile + test run. I still have no idea why my environment would be contributing to this.
|
Interesting result on the 3rd custom test. That's helpful. Looks like there is still a conversion problem showing up in the object test too. Thanks. I'll have another attempt tonight. |
Pushed a version with a few more print statements. |
Debugging via github comments continues...
|
Ok, made some additional changes. Might have fixed some. |
Success!?
|
Better than I hoped for. Thanks. I'll clean up the debug stuff and make a release. |
I looked at the diff but it's over my head. What was the root cause? And were you able to determine why this was not discovered by existing tests or Windows CI? |
There were two issues. One is that Windows thinks long integers are 32 bits when converting from Ruby but oddly not at other times. Using Second was a |
Okay to close now that the release has been out for a few weeks? |
Fine by me, 3.6.5 seems to work fine and I'm sure I'll make a new issue if I find anything :D |
I'm seeing an error dumping a Time value. The error is:
I tested the following configurations:
error with:
no error with
simple repro:
The text was updated successfully, but these errors were encountered: