-
Notifications
You must be signed in to change notification settings - Fork 378
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
Size matchers to provide insightful debugging information #294
base: master
Are you sure you want to change the base?
Conversation
This is an interesting idea. The reason we didn't show the actual collection contents originally is that we had no idea how big it would be (could be very large in some cases), so we left it out. Also, where there are compound matches on a collection, each one might dump out the whole contents. I can see your point, but I'm not convinced that the removed duplication is worth the extra code. My experience is that the flakiness of the Java APIs sometimes justifies some clumsy code. |
Cool, so if you agree that it is a concern, shall we add a truncation logic as I was suggesting in #174 ?
You mean the SizeMatcher? You prefer duplicating the code in every IsXXXWithSize class?
My opinion is that if you want to re-think the design, I'm up for it. But until then, let's stick with what is already in place. |
@alb-i986 looks good, i've created a custom matcher myself, so the output reports what is in the collection is it is the wrong size. please can you rebase from master, as |
Now, matchers like hasSize and similar, in case of mismatch, print also the actual values contained in the collection being tested. Closes hamcrest#174
Rebased and squashed. Ready to be merged. |
Going to try and kick start hamcrest, so if you want to get it merged, please rebase from the branch |
9bc653b
to
e9f7fc8
Compare
Resolving my own #174.
I'm leaving the commits separate for easier reviewing, I'll squash them afterwards.
Briefly, the solution introduces a base class (I called it
SizeMatcher
) for the matchers which match on the size of a collection-like object.Although, I have to say, I'm not a huge fan of complex hierarchies (low coupling, you know), but it made perfect sense to align with the current design. Maybe that could be a separate topic to discuss, if you think it's something which could be discussed.
One thing to note is the naive algorithm I'm using to generate the correct indefinite article "a"/"an", in
SizeMatcher#indefiniteArticle
.I guess you must have already bumped into this problem, so if you already have a method to solve it more elegantly, I'll be more than happy to use it.
Please check the commit messages for more details on the changes (I had to update a few tests).
Thanks
PS: here I didn't take into account the "problem" for huge collections, which I was mentioning at the end in #174. I think we should not worry about that. The output may get cluttered, but that's the way it is, if you have a huge collection. Not our problem.