Nimble
Nimble copied to clipboard
Protocol Extension Discoverability
Would there be any interest in updating Nimble's matchers to work using protocol extensions for discoverability?
E.g.,
// before
expect(1 + 1).to(equal(2))
expect(1.2).to(beCloseTo(1.1, within: 0.1))
expect(3) > 2
expect("seahorse").to(contain("sea"))
expect(["Atlantic", "Pacific"]).toNot(contain("Mississippi"))
expect(ocean.isClean).toEventually(beTruthy())
// after
expect(1 + 1).to.equal(2)
expect(1.2).to.beCloseTo(1.1, within: 0.1)
expect(3).to.beGreaterThan(2) // more verbose, but discoverable
expect("seahorse").to.contain("sea")
expect(["Atlantic", "Pacific"]).toNot.contain("Mississippi")
expect(ocean.isClean).toEventually.beTruthy()
Having smarter auto-completion can be invaluable when first using a project and can help users discover APIs that they may have otherwise never used.
Definitely agree! This changes the custom matcher interface, which means a major version bump for Nimble. Still, I think the improvement in autocompletion is well worth it.
Yeaaaaahhhhhhhhhhh
It might not even have to be a breaking change immediately. We could add the protocol extension layer and call the module functions from there. The question is if Swift will resolve func to()
vs. var to
.
It might not even have to be a breaking change immediately. We could add the protocol extension layer and call the module functions from there.
Brilliant! :100:
If no one else wants to noodle on this, I can take a stab this weekend :smile:
Did a quick look at this and it doesn't look like var to
and func to()
can live in harmony :disappointed:
I see two paths forward…
-
The backwards-compatible:
// current style expect(array).to(beEmpty()) expect(array).toNot(beEmpty()) // new style expect(array).isEmpty() expect(array).isNotEmpty()
-
The breaking:
expect(array).to.beEmpty() expect(array).toNot.beEmpty() // or: expect(array).to.not.beEmpty()
Thoughts? The latter seems to fulfill the expectations (har har) of someone familiar with BDD, though the former is more succinct and active, kind of like changing a spec from it "should do this"
—to: it "does this"
.
It's always possible to use something else other than to
Sure. Ideas welcome if isEmpty()
/isNotEmpty()
and their ilk aren't desirable! Even is
might work:
expect(array).is.empty()
expect(array).is.not.empty()
expect(foo).is.equalTo(bar)
expect(foo).is.not.equalTo(bar)
What about no backwards-compatibility and a major dot release? Just an idea, feel free to push back on it.
I'm cool with that. And on second thought is
is a keyword, so it's not a friendly option. I'll try a breaking-change PR soon.
Well, I mean, whatever @jeffh thinks is a good idea. I'm just throwing it out there. Seems like we have to compromise on the API to be backwards-compatible, so what does it look like if we remove that constraint?
@modocache: I just don't see a big upside for the breaking change for existing users. I'm not against changing the entire API, but for what amounts to syntax change (with little end-user benefit), it doesn't seem as critical to break existing users. While I do understand the visual appeal of the dot-styled syntax and it's autocomplete benefits, I need more convincing to break every user of Nimble.
Maybe I just don't see it, but I'm not visualizing unique features, significantly better code (maintenance) as a benefit to this change. For example, an alternative to writing combinators:
// Adding `or` combinator to the existing system
expect(something).to(equal(2) || equal(4))
While I haven't really thought through it enough to make an opinionated stance to support combinators, the existing system makes it easy to extend towards that. But maybe enough people don't care about that kind of feature to be worth effort.
If there's something significantly more advantageous to doing dot-styled syntax, I could be convinced for a drop-the-world syntax replacement. Making (thousands of) developers to spend several hours updating their expectations is wasted engineering I prefer not to do at a drop of a hammer. I'd rather to a gradual migration instead (in parallel for several releases before removing the old).
Also, it needs more thorough evaluation based on what the swift compiler can handle (these existing generics have been painful to extend because of it).
Now my 2 cents about the syntactic style.
The first style:
expect(foo).isEmpty()
expect(foo).isNotEmpty()
Looks pretty expensive to implement and maintain. We'd need four methods for every matcher-expectation combination (to
, toNot
, toEventually
, toEventuallyNot
). Correct me if I'm wrong, but I'm not too thrilled about that.
The second style is much more to my liking overall. There are possible other styles, just throwing some ideas:
-
expect(x).should.equal(y)
(old-rspec; gomega) -
it(x).should.equal(y)
- We'd have to make sure overloading works between Quick & Nimble -
then(x).will.equal(y)
,then(x).should.equal(y)
,then(x).is.equalTo(y)
- more cucumber-like
@jeffh I'm happy to work on a backwards-compatible version, which is why I suggested it as a good first step :)
I may be in the minority, but I do think these kinds of changes go beyond syntax and visual appeal, and that the autocompletion benefits are invaluable to new users—especially new developers. I think Swift 2 introduced protocol extensions as an important step in its evolution as a language.
I also agree with you that using some kind of negating not
(should
vs. should.not
) would be easier both maintainability- and discoverability-wise. It's just about finding the sweet spot, semantically.
I just read the combinator thread and might need more examples to understand the need for this kind of binary expectation. Instead of:
expect(someThing).to(startWith("https://")).or(startWith("http://"))
Why not the following?
expect(someThing).should.match("https?://")
Turns out it's a moot point. My earlier spike quickly led to an unhelpful compile-time error that was hiding the root of the problem. Both can live in harmony after all! :smile:
Sorry for the slow response. Still recovering from an illness :frowning:.
#176: Was requesting matching IPv6 or IPv4 addresses (and not just by http(s)). While you can use a conditional regex, it gets far less pretty:
expect((device.primaryIPAddress =~ RegularExpressions.IPv4Pattern) || (device.primaryIPAddress =~ RegularExpressions.IPv6Pattern)).to.beTruthy()
A matcher framework like Hamcrest uses combinators to build a feature-rich matcher library. It's more flexible at the cost of less discoverability (because it's all function composition):
// java
assertThat(responseString, either(containsString("color")).or(containsString("colour")));
In my opinion, good architecture of software is keeping the most options available regardless about what design patterns are used. I understand people love protocol extensions as a new paradigm, but it seems mostly the same to me -- granted, with better discoverability semantics. Just saying it's an evolution to a language is interesting to note, but it's just an appeal to authority. Language designers can make mistakes too.
Here's what I've understand around the tradeoffs of the dot-syntax:
Pros:
- It's less syntactically noisy
- It's more discoverable for new users (eg - autocomplete)
Cons:
- Disallows the creation of matcher combinators
@stephencelis, that's for discovering that both can live side by side. It's good to hear :+1:.
I don't see any reason why protocol extension-based syntax would disallow matcher combinators.
As an example:
expect(device.primaryIPAddress).to.match(IPv4Pattern).or.match(IPv6Pattern)
If we look at that, it's ambiguous meaning:
// a.
expect(device.primaryIPAddress).to.match(IPv4Pattern)
// b.
expect(device.primaryIPAddress).to.match(IPv4Pattern).or.match(IPv6Pattern)
How does Nimble know which form (a or b) is executing? Is it ok to perform an assertion during the invocation to match
? That would invalidate the purpose of the or
. Alternatively, you'll need some wrapping call around the grammar (or only use operators):
// || operator takes both args as autoclosures to capture both matchers:
expect(device.primaryIPAddress).to.match(IPv4Pattern) ||
expect(device.primaryIPAddress).to.match(IPv6Pattern)
A combinator needs to control when it's child matchers execute. I don't think that syntax can cover it.
Swift type-checking could take care of it and correctly route things. The assertion would build lazily and be performed at the end of the statement.
That requires double the implementations for matcher writers. But I think you've convinced me enough. I'll merge that PR once it's a good state :+1:
:smile: Yay! Have you had a chance to read over #218 and address the concerns I brought up?
Hey @stephencelis,
Sorry for worst turn-around on record. I'll look to try and get the PR up-to-date in the coming weeks.
I'm still interested in getting this into Nimble. I was wondering what were you're thoughts on getting satisfyAnyOf
to function as a protocol extension?
No problem! Open source should ideally be fun and low-stress (which I know open threads can rarely be). If you get this across the finish line, great!
I wasn't familiar with satisfyAnyOf
, but imagine the ||
version would still work:
expect(82).to.beLessThan(50) || beGreaterThan(80)
And, more verbosely (without the free function):
expect(n).to.beLessThan(50)
|| expect(n).to.beGreaterThan(80)
At this point, I don't think this is worth doing. It might have been more useful years earlier in the project. Introducing this much of a syntax change to Nimble, nearly 9 years since its original release, would not be worthwhile.