Fixed integer overflows for Foundation extensions

[soxjke]

• Fixed integer overflows for Foundation extensions (Integer overflow with timer function on specifying seconds interval greater than 21474.836471 #506) …
• Added overflow test case
• Removed note on value limits for DispatchTimeInterval * operator

Checklist

• Updated CHANGELOG.md.

[andersio]

Seems like the test has failed to be built on Linux.

/home/travis/build/ReactiveCocoa/ReactiveSwift/Tests/ReactiveSwiftTests/FoundationExtensionsSpec.swift:102:59: error: cannot invoke 'equal' with an argument list of type '(DispatchTimeInterval)'
 expect(DispatchTimeInterval.seconds(Int.max) * 10).to(equal(DispatchTimeInterval.never))
 ^
/home/travis/build/ReactiveCocoa/ReactiveSwift/Tests/ReactiveSwiftTests/FoundationExtensionsSpec.swift:102:59: note: overloads for 'equal' exist with these partially matching parameter lists: (T?), ([T : C]?), ([T]?), ([T?]), (Set<T>?)
 expect(DispatchTimeInterval.seconds(Int.max) * 10).to(equal(DispatchTimeInterval.never))
 ^

[andersio]

Please start the line with 1. .

[andersio]

Thank you for your work. 😸

[soxjke]

Coming up with linux fix 👍

[soxjke]

@andersio also if you don't mind I suggest changes to 3 particular test case expressions within this file. From my point of view, expect.to.equal syntax explicitly expresses expectation of equality when == operator makes people get inside Nimble and look for necessary overload to make sure whether expression is not false-positive.

[andersio]

I thought this is Equatable based, and the stdlib floating point types already imply IEEE 754 compliant equality for ==.

Edit: Could you elaborate on why this has come up in your mind?

makes people get inside Nimble and look for necessary overload to make sure whether expression is not false-positive.

[soxjke]

Sure, the idea is following:

Using any kind of spec + expectation/matchers framework: (Kiwi, Specta/Expecta, Quick/Nimble) I expect the following pseudo-syntax line for test assertion:

expect(inspectable-expression) -> kind-of-expectation(to/notTo, eventually, eventuallyBeforeTimingOutAfter..) -> matcher-expression

All of mentioned above frameworks provide us with some DSL to set test expectations, so as a developer I expect an expectation (sorry for tautology) to be set using DSL. With Expecta I can write expect(someInt) == 0, and it won't warn me because I can compare returned object with 0 in Objective-C. But the test flow will be wrong, because it won't verify this against any matcher. Instead I should write expect(someInt).to.equal(0), which is correct in terms of its DSL.

Seeing expect((-DispatchTimeInterval.never).timeInterval) == Double.infinity in Swift makes me find whether its valid DSL syntax first. I get to https://github.com/Quick/Nimble/blob/master/Sources/Nimble/Matchers/Equal.swift#L172 and find that there's an implementation of == operator which makes lhs.to(equal(rhs) that I expected in the first place.

Maybe my confusion is connected with unsafe Objective-C background, where equality comparisons could be done mistakingly with reference arithmetics, and we should disregard it, however I would still wonder why to use operator == over matcher syntax for particular equality case. Especially, if this operator is implemented by calling matcher syntax.

[andersio]

== are the fluent conveniences of the matchers that are meant to help reduce the verbosity AFAIK. It could be confusing for Swift newcomers with C family language background, but IMO it does make sense given equality and inequality in Swift are implemented as operators, with compile time overloading & type safety guarding for the problems you may otherwise encounter.

[soxjke]

No problem, bringing == syntax back. I don't have that strong opinion regarding this readability case, so knowing that's just a convenience matcher makes me fine with this approach. Also, I've checked Nimble matchers README and they show using operator == for equality matcher and === for identity matcher. Thus this point might be considered resolved?

[soxjke]

@andersio
I'm starting to receive

Test Case 'XCTestCase.MutableProperty, should not deadlock' started at 2017-10-23 08:37:59.778

for Linux build from Travis. Could you probably advice because I'm a bit stuck with what's going on with it...

[andersio]

@soxjke That's an intermittent issue. Restarted the test. :(

[soxjke]

@andersio thanks!

I opened #544 to keep track on it. Probably, I'll run vagrant on a local machine to simulate this over weekend.

Other than that, do you see any changes needed more for this PR?

[NachoSoto]

Thanks!

Do you mind squashing/removing the last commit? 🙏

[soxjke]

@NachoSoto Sure, I've made overall feature history prettification and dropped also two "Update CHANGELOG.MD" commits as well. Hope Linux tests will succeed this time :)

[soxjke]

Probably need to re-run tests again, https://travis-ci.org/ReactiveCocoa/ReactiveSwift/jobs/291677337 suffers from the same failure as #544