Replaced XCTAssertArray(Float|Double)EqualWithAccuracy(_:_:accuracy:) …

[regexident]

… with short-circuiting overload of XCTAssertEqual(_:_:accuracy:)

The current version floods the console (especially var n = 100_000) with assertion failures, bringing Xcode to its knees.
This modification short-circuits on first failure.

Unlike the current version it also preserves #line and #file.

It also overloads XCTAssertEqual(_:_:accuracy:), eliminating the need for XCTAssertArray(Float|Double)EqualWithAccuracy(_:_:accuracy:).

[mattt]

Thanks for this, @regexident. The functionality this is providing is helpful and strongly-motivated, and really well-written. I was pleasantly surprised to see Matrix conformance to ExpressibleByArrayLiteral and Collection in here too.

I only have one top-level comment (which may amount to bike-shedding, so apologies in advance if that's indeed the case):

The current spelling of XCTAssertEqual is most similar to the built-in generic comparator assertion, but it most resembles XCTAssertEqualWithAccuracy, which makes me wonder if it should be named closer t the latter. I think I prefer having accuracy as a labeled parameter, rather than being implicit from the head of the function name, but then again, both of those are effectively bridged functions from C macros, so standard Swift API naming conventions seem to be out the window (which makes me wonder if it's worth distinguishing our custom Surge-specific assertion from the built-in XCT ones...)

I don't have a very strong feeling any which way, but right now the spelling I think I most favor is something like XCTAssertEqualFloatingPointElementsWithAccuracy for a top-level function, or validateEqualElements if we decide to define a helper function in an extension on XCTestCase.

What's your take with all of this, @regexident?

[mattt]

Mismatch reference to XCTAssertEqualWithAccuracy in failure message.

[regexident]

Curiously enough XCTAssertEqual(_:_:accuracy:file:line:) will produce an error message like seen above, referencing XCTAssertEqualWithAccuracy. I copied it from an intentionally failed test.

import XCTest
@testable import Foobar

class FoobarTests: XCTestCase {
    func testExample() {
        // Test error: XCTAssertEqualWithAccuracy failed: ("1.0") is not equal to ("0.9") +/- ("0.01")
        XCTAssertEqual(1.0, 0.9, accuracy: 0.01)
    }
}

[regexident]

XCTAssertEqualWithAccuracy(_:_:accuracy:_:file:line:) seems to have been deprecated in Xcode 9 in favor of XCTAssertEqual(_:_:accuracy:file:line:), which is why I chose to name it accordingly.

import XCTest
@testable import Foobar

class FoobarTests: XCTestCase {
    func testExample() {
        // Compiler warning:
        // 
        // 'XCTAssertEqualWithAccuracy(_:_:accuracy:_:file:line:)'
        // is deprecated: renamed to 'XCTAssertEqual(_:_:accuracy:file:line:)'
        XCTAssertEqualWithAccuracy(1.0, 0.9, accuracy: 0.1)
    }
}

[regexident]

Commit 2363135 refactors the implementation of XCTAssertEqual for 1D arrays and 2D grids.

By default XCTest provides the following implementations:

	T: FloatingPoint	T: [FloatingPoint]	T: [[FloatingPoint]]
XCTAssertEqual<T>(_:_:_:file:line:)	🔳	🔳	🔳
XCTAssertEqual<T>(_:_:accuracy:_:file:line:)	🔳	⬜️	⬜️

Unfortunately all these do is call lhs == rhs and fail with …

XCTAssertEqual failed: (4.0) is not equal to (42.0)

… for scalar values.

XCTAssertEqual failed: ("0.0	1.0	2.0	3.0	4.0	5.0	6.0	7.0	8.0	9.0") is not equal to ("0.0	1.0	2.0	3.0	42.0	5.0	6.0	7.0	8.0	9.0")

… for vector values.

XCTAssertEqual failed: ("⎛	0.0	1.0	2.0	⎞
⎝	3.0	4.0	5.0	⎠
") is not equal to ("⎛	0.0	1.0	2.0	⎞
⎝	3.0	42.0	5.0	⎠
")

… for matrix values.

In increasing dimensionality this becomes less and less human readable.

---

The Surge-provides overloads of XCTAssertEqual(_:_:accuracy:_:file:line:) filling the above gaps allowing us to compare multi-dimensional tensor values within a certain accuracy:

	T: FloatingPoint	T: [FloatingPoint]	T: [[FloatingPoint]]
XCTAssertEqual<T>(_:_:_:file:line:)	⬜️	⬜️	⬜️
XCTAssertEqual<T>(_:_:accuracy:_:file:line:)	⬜️	🔳	🔳

The current implementation of these bring their own problems though:

Instead of providing too much information in the failure message they provide too little:

As such in current Surge a test like this …

let actual: Vector<Float> = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
let expected: Vector<Float> = [0, 1, 2, 3, 42, 5, 6, 7, 8, 9]

XCTAssertEqual(actual, expected)

… would simply fail with an error message like this …

XCTAssertEqual failed: (4.0) is not equal to (42.0)

---

Which is why I refactored it to fail with a neat index hint like this …

XCTAssertEqual failed: Failure at index [4]: (4.0) is not equal to (42.0) +/- (1e-05)

… in case of 1D vector-like input, or this …

XCTAssertEqual failed: Failure at index [4, 1]: (4.0) is not equal to (42.0) +/- (1e-05)

… in case of 2D matrix-like input.

Unfortunately this ergonomics improvement only applies to the Surge-provided higher-dimensional versions of XCTAssertEqual<T>(_:_:accuracy:_:file:line:), leaving XCTAssertEqual<T>(_:_:_:file:line:) just as un-ergonomic as it ever was.

---

Which is why I added explicit aliases XCTAssertEqual1D and XCTAssertEqual2D, both with and without accuracy, which properly provide the improved ergonomics.

The documentation explains the difference:

///  Semantically the same as its `XCTAssertEqual<T>(_:_:accuracy:_:file:line:)` counterpart
///  (i.e. without the `…1D` suffix), but with improved error messages.

Long story short: With commit 2363135 your existing tests using Surge-provided asserts get improved ergonomics without any change necessary, while for those provided by Apple you need to use the alternative …1D, …2D variants to have it resolve to the improved implementation. There is no …0D variant for scalar values as those are fine as is.