Skip to content

[TestJSONEncoder] Re-enable tests and add a special case for testEncodingDate #23721

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

Merged

Conversation

bendjones
Copy link
Contributor

Handles a rare condition caused by snprintf's %g which JSONSerialization uses internally for double values. This bug effects Darwin, FreeBSD, and Linux currently.

@bendjones
Copy link
Contributor Author

@swift-ci smoke test

@bendjones bendjones force-pushed the bendjones/TestJSONEncoder_fixes branch from 2fdde84 to 6aed5ee Compare April 3, 2019 15:29
@bendjones
Copy link
Contributor Author

@swift-ci smoke test

Copy link
Contributor

@itaiferber itaiferber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me overall but I think we need to bail before the expectEqual check, since it appears to be unconditional right now (so won't get us out of what we're trying to avoid here).

@bendjones bendjones force-pushed the bendjones/TestJSONEncoder_fixes branch from 6aed5ee to 2b88f6b Compare April 4, 2019 17:58
@bendjones
Copy link
Contributor Author

@swift-ci smoke test

@bendjones bendjones requested a review from itaiferber April 4, 2019 20:01
@itaiferber
Copy link
Contributor

Ben, I realize it might be worth adding an explicit test along with this (inline) not just for Date() but for a fixed date for which we know would be a problem, just to assert that the workaround is valid. But, not specifically necessary here just to get this stuff passing

@bendjones
Copy link
Contributor Author

@itai agreed... let me add a counter test here to ensure my new test works as expected. I can pull from the playground.

…rare condition caused by `snprintf`'s `%g` which `JSONSerialization` uses internally for double values. This bug effects Darwin, FreeBSD, and Linux currently.
@bendjones bendjones force-pushed the bendjones/TestJSONEncoder_fixes branch from 2b88f6b to c4ee047 Compare April 4, 2019 20:32
@bendjones
Copy link
Contributor Author

@swift-ci smoke test

}

// Test the above `snprintf` edge case evaluation with a known triggering case
let knownBadDate = Date(timeIntervalSinceReferenceDate: 0.0021413276231263384)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itaiferber added this case which will always pass through the above edge case

@bendjones
Copy link
Contributor Author

Cool merging!

@bendjones bendjones merged commit 9c0de5a into swiftlang:master Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants