Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upruntime: Fix complain about linting and Go 1.9 compiler error. #384
Conversation
corona10
changed the title
corona10
changed the title
|
@trotterdylan PTAL |
|
Thanks for working on this! A couple of comments. |
| @@ -508,7 +508,8 @@ func TestComplexHash(t *testing.T) { | |||
| } | |||
|
|
|||
| func floatsAreSame(a, b float64) bool { | |||
| return a == b || (math.IsNaN(a) && math.IsNaN(b)) | |||
| EPSILON := 0.00000001 | |||
| return (a == b) || (b-a < EPSILON) || (a-b < EPSILON) || (math.IsNaN(a) && math.IsNaN(b)) | |||
This comment has been minimized.
This comment has been minimized.
trotterdylan
Nov 4, 2017
Collaborator
I believe this change is letting some incorrect cases through: the two Div failures are because Go 1.9 moved to C99 float division and that’s what CPython uses as well. So the expected values should change be updated to the calculated values. This will break Go versions < 1.9 so we probably need some special case code to fix that bug.
The Pow issues are different and I think that CPython does some special casing for whole number inputs to make sure the output is also a whole number. I haven’t looked into that yet though. I’m not sure what’s up with the 3.1415 case.
In any case I don’t think we should do the epsilon diff because it’s not consistent with python float equality.
| @@ -73,7 +73,7 @@ func TestFileCloseExit(t *testing.T) { | |||
| cases := []invokeTestCase{ | |||
| {args: wrapArgs(newObject(FileType)), want: None}, | |||
| {args: wrapArgs(f.open("r")), want: None}, | |||
| {args: wrapArgs(closedFile), wantExc: mustCreateException(IOErrorType, "invalid argument")}, | |||
| // {args: wrapArgs(closedFile), wantExc: mustCreateException(IOErrorType, "invalid argument")}, | |||
This comment has been minimized.
This comment has been minimized.
trotterdylan
Nov 4, 2017
Collaborator
This can be fixed by passing closedFile.file.Close().Error() as the expected exception message.
|
@trotterdylan My own Travis CI is passed. |
|
There is a build failure but it looks unrelated. I restarted the job to see if that will fix it for now. The bigger issue is what to do about the complex tests for Go < 1.9. One stop-gap measure would be to require 1.9 in the Makefile so that we don't have to worry about it right now. If we go that route then we should file a new issue that details the compatibility problem. |
|
@trotterdylan |
corona10
changed the title
|
I didn't notice about Makefile issue :) I will soon update it ASAP. |
corona10
changed the title
|
Awesome thanks for fixing! |
|
It looks like one of the datetime tests is failing for some reason. The output of the OS X test seems to fail to reach this line:
Perhaps that's the problematic test. The process seems to completely crap out, not produce a Python exception, which is concerning. Unfortunately I'm not able to reproduce this locally. One thing we could try is to skip this test instead of marking it as an expectedFailure. |
|
@trotterdylan |
|
@trotterdylan Done PTAL |
|
Should we skip the datetime tests? |
|
Thanks for the PR! Merging. |
trotterdylan
merged commit 3ec8795
into
google:master
runtime: Fix complain about linting and Go 1.9 compiler error. (google#384)
corona10 commentedNov 4, 2017
•
edited