Skip to content
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

Go1.23 first-class iterator support in certain collection-related matchers #795

Closed
9 tasks done
thediveo opened this issue Nov 10, 2024 · 5 comments
Closed
9 tasks done

Comments

@thediveo
Copy link
Collaborator

thediveo commented Nov 10, 2024

Now that Go 1.23 comes with (push) iterators, after playing around with some of my own code I notice ending up writing tests like this one...

foos := []Bar
for _, foo := range Bars() {
    foos = append(foos, foo)
}
Expect(foos).To(ConsistOf("foo", "bar", "baz"))

...but I would rather avoid iterator-related pickup boilerplate:

Expect(Bars()).To(ConsistOf("foo", "bar", "baz"))

Do you think that first-class support would make sense (even if some work), where matchers that are currently handling collections for their actual value, like slice and map, in a near future additionally handle iter.Seq[V] and iter.Seq2[K, V] right out of the box?

  • iter.Seq and iter.Seq2 actuals, where only the V values are taken into consideration in case of iter.Seq2:
    • BeEmpty
    • HaveLen
    • HaveEach
    • ContainElement
    • HaveExactElements
    • ContainElements
    • ConsistOf
  • only with iter.Seq2 actuals
    • HaveKey
    • HaveKeyWithValue

The go.mod would be at most at 1.22, with the new iterator-awareness hidden behind a go:build go1.23 build constraint; the challenge is rather how to pimp the existing matchers without ending up in two completely different code paths that need to be maintained at least until go 1.24 will be out. Now, it would be natural to refactor the existing collection-related matchers to use iterators ... dang!

Any(thing) missing? Any suggestions? Any groans? 😁

@thediveo
Copy link
Collaborator Author

I'm now working on this and ContainElement has gotten iterator-awareness, including partial test case coverage. ContainElement is optimized in that it does not simply loop over all elements to create a slice and then fall back to work on this slice, but instead does the checking directly on the iterator yields.

Probably HaveEach can also be optimized as well, but some other matchers might need to use the intermediate slice.

@onsi
Copy link
Owner

onsi commented Nov 13, 2024

hey sorry for the radio silence on my end - this is fantastic! thank you 😊

@thediveo
Copy link
Collaborator Author

atm only in my fork, will send PR when there's more

@thediveo
Copy link
Collaborator Author

thediveo commented Nov 20, 2024

update: now only two collection-related matchers remain to be done, ContainElements and ConsistOf are the oddballs, in that they need to collect all elements produced by an actual iterator in order to then feed into the bisect algorithm. And I'm already having fun writing code at work using iterators, where they fit it.

@thediveo
Copy link
Collaborator Author

you've #798, please have a good look at it that it does actually meet your expectations; tests all pass. The now iterator-aware matchers will also mention expecting iter.Seq and iter.Seq2 when build with Go versions before 1.23. The rationale is that I didn't want to maintain two separate error messages based on version, but drop me a note if you want different. Other than that, when build with a Go toolchain before 1.23 iterator-assignable functions will simply be rejected. Only when building with 1.23 or later, the iterators will fully accept iterators.

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

No branches or pull requests

2 participants