General index consistency tests

[timvermeulen]

This is a stab at making the index tests from ChainTests available to other collections as well (for now just chain and product). validateIndexTraversals validates a collection by taking an array of its indices as the source of truth, and verifying that all index operations (with just about any combination of arguments possible) are consistent with these indices.

validateIndexTraversals doesn't just use c.indices + [c.endIndex] as the indices to test against because that might not catch a bug in the implementation of indices itself (or e.g. index(after:) or Index.==). Having to provide an array of indices that are known to be correct prevents these kinds of bugs from slipping through. As an example, the index array of a Chain2 instance is computed like this:

chain.base1.indices.map { .init(first: $0) }
  + chain.base2.indices.map { .init(second: $0) }
  + [.init(second: chain.base2.endIndex)]

The previous bugs in Chain2 and Product2 would all have been caught by this, so this would give me a bit more confidence before attempting Product2.index(_:offsetBy:) 🙂

Checklist

• I've added at least one test that validates that my change is working, if appropriate
• I've followed the code style of the rest of the project
• I've read the Contribution Guidelines
• I've updated the documentation if necessary

[karwa]

FWIW I've been using this to check collection consistency: https://github.com/karwa/swift-checkit/blob/master/Sources/Checkit/CollectionChecker.swift

I posted about it on the forums - I think it would be a super-useful thing to have a package of generic code which exercises standard library protocols in order to check semantics. For example, it's also useful to check that successive indexes all compare as greater than the last, that empty collections do the right thing, etc.

[kylemacomber]

@karwa I totally agree! Within the Swift we rely on suite of tests to validate all of the collections in the Standard Library: https://github.com/apple/swift/tree/main/stdlib/private/StdlibCollectionUnittest. It'd be awesome to have package version of this code.

[timvermeulen]

For example, it's also useful to check that successive indexes all compare as greater than the last, that empty collections do the right thing, etc.

@karwa Both of these things are covered by validateIndexTraversals, or did you have something specific in mind that it doesn't test?

I would also like to see this as a separate package! That probably still shouldn't stop us from pursuing this in the short term? There's a pretty direct need for this in the chain and product tests, and the upcoming interspersed(with:), striding(by:), and slidingWindows(ofCount:) methods will greatly benefit as well from tests that go beyond basic iteration.

One thing I would like to see in whatever we end up with is a way to also pass indices to test against, like I do here. Having consistency between the index methods is a good start but it doesn't show that the collection does what it says it does, e.g. Chain2 could produce the product of two collections instead and still pass all consistency tests. Having said that, it's probably a good idea to make that parameter optional and fall back on c.indices + [c.endIndex] in case there's no straight-forward way to compute the collection's indices independently of its implementation.

[natecook1000]

This is great, @timvermeulen! I'd love to see this as a separate package eventually (including things like verifying performance for random-access collections, where possible), and it's great to start here.

[natecook1000]

I find it a bit confusing that this shares the name of c.indices but has one more element — could this be allIndexes / allValidIndices / indicesIncludingEnd, etc?

[timvermeulen]

Great idea, I'll go with indicesIncludingEnd. This also makes the - 1 on the next line less confusing.

[natecook1000]

I think we need a check that c.count equals c.indices.count here.

[timvermeulen]

Nice find!

[karwa]

I would also like to see this as a separate package! That probably still shouldn't stop us from pursuing this in the short term?

I mean, there is a package already - swift-checkit. I posted about it here, but despite the positive reception nobody has sent any PRs.

It also has a couple of other things - a SinglePassSequenceChecker for generic algorithms, because most of them never get tested with a Sequence that truly only supports iteration one time, and some basic floating-point checks (I'm not a numerics expert, but people do try and write their own float types and often ask about what the IEEE standards require)

I would be very happy if anybody wanted to fork it, improve it, or use it as part of another test package. It's open-source, after all.