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

new: make collection-related matchers Go 1.23 iterator aware, resolves onsi/gomega#795 #798

Merged
merged 1 commit into from
Nov 24, 2024

Conversation

thediveo
Copy link
Collaborator

  • new internal helper package for dealing with Go 1.23 iterators via reflection; for Go versions before 1.23 this package provides the same helper functions as stubs instead, shielding both the matchers code base as well as their tests from any code that otherwise would not build on pre-iterator versions. This allows to keep new iterator-related matcher code and associated tests inline, hopefully ensuring good maintainability.
  • with the exception of ContainElements and ConsistOf, the other iterator-aware matchers do not need to go through producing all collection elements first in order to work on a slice of these elements. Instead, they directly work on the collection elements individually as their iterator produces them.
  • BeEmpty: iter.Seq, iter.Seq2 w/ tests
  • HaveLen: iter.Seq, iter.Seq2 w/ tests
  • HaveEach: iter.Seq, iter.Seq2 w/ tests
  • ContainElement: iter.Seq, iter.Seq2 w/ tests
  • HaveExactElements: iter.Seq, iter.Seq2 w/ tests
  • ContainElements: iter.Seq, iter.Seq2 w/ tests
  • ConsistOf: iter.Seq, iter.Seq2 w/ test
  • HaveKey: iter.Seq2 only w/ test
  • HaveKeyWithValue: iter.Seq2 only w/ test
  • updated documentation.

- new internal helper package for dealing with Go 1.23 iterators
  via reflection; for Go versions before 1.23 this package provides
  the same helper functions as stubs instead, shielding both the
  matchers code base as well as their tests from any code that
  otherwise would not build on pre-iterator versions. This allows
  to keep new iterator-related matcher code and associated tests
  inline, hopefully ensuring good maintainability.
- with the exception of ContainElements and ConsistOf, the other
  iterator-aware matchers do not need to go through producing all
  collection elements first in order to work on a slice of these
  elements. Instead, they directly work on the collection elements
  individually as their iterator produces them.
- BeEmpty: iter.Seq, iter.Seq2 w/ tests
- HaveLen: iter.Seq, iter.Seq2 w/ tests
- HaveEach: iter.Seq, iter.Seq2 w/ tests
- ContainElement: iter.Seq, iter.Seq2 w/ tests
- HaveExactElements: iter.Seq, iter.Seq2 w/ tests
- ContainElements: iter.Seq, iter.Seq2 w/ tests
- ConsistOf: iter.Seq, iter.Seq2 w/ test
- HaveKey: iter.Seq2 only w/ test
- HaveKeyWithValue: iter.Seq2 only w/ test
- updated documentation.

Signed-off-by: thediveo <[email protected]>
@onsi
Copy link
Owner

onsi commented Nov 21, 2024

wonderful :) will take a close look today/tomorrow. thanks for this!

@onsi onsi merged commit 4c964c6 into onsi:master Nov 24, 2024
6 checks passed
@onsi
Copy link
Owner

onsi commented Nov 24, 2024

hey this is fantastic - thank you. it's clear a lot of care, time, and thought when into both the code and the documentation and I really appreciate that! 🤩

i've merged it in. The only thought I had, looking through it, is around output when the matchers fail. For example, if you pass ContainElement a slice Gomega will spit out the whole slice along with the message "to contain X". My sense is that seeing the whole slice is often helpful for understanding the failure.

I believe the new code would emit the iterator function instead of the returned elements. In part this is a good thing: no need to collect all the elements to make an assertion, especially when you don't need them all to succeed/fail. But I wonder how it'll play out as folks start using it in anger.

None of that needs to be changed today. I say we ship this and get it out there. If folks start opening issues and/or we start to bump into this pain signal we can consider subsequent improvements. Sound good? You good with me cutting a release soon?

@thediveo
Copy link
Collaborator Author

Yes on all your points!

There are different ways to alleviate:

  • Go's "make it explicit" attitude
    • either standard (x) lib helpers to reduce iters to slices or maps before passing the latter as actual,
    • Gomera giter convenience package using generics
  • doing the slice/map under the hood, either partial or completely
  • keeping the already iterated elements to provide better details up to the point of failure.

@thediveo
Copy link
Collaborator Author

and yes, please cut a release if you please 😀

@onsi
Copy link
Owner

onsi commented Nov 25, 2024

done! and makes sense on the ways to alleviate. Thanks for this super valuable contribution!

@thediveo thediveo deleted the feature/iter branch December 6, 2024 19:08
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

Successfully merging this pull request may close these issues.

2 participants