Add docs for new array matchers #1210

lucasfcosta · 2016-12-20T13:11:16Z

Purpose (TL;DR)

This aims to add docs for each one of the changes made at #1208 (which adds new Array Matchers).

Solution

I've read the CONTRIBUTING.md file in the docs folder and I think this is the correct place to add the new docs, but I'm not 100% sure, so please correct me if I'm wrong and I'll add these pieces of information to the right place.

Also, I'm not a native english speaker so please let me know if you find any spelling/grammar mistakes. It would be much appreciated 😄

PS: I've made this in a single commit, but I can split it up into multiple commits, one for each matcher, if you find it to be better.

How to verify

Check out this branch (see github instructions below)
Go to the docs folder by running cd docs on Sinon's root folder
Ensure you have bundler by running gem install bundler
Install dependencies by running bundle install
Run a local server by running bundle exec jekyll serve
Go to http://localhost:4000/releases/v2.0.0-pre.4/matchers/ and check the new docs

coveralls · 2016-12-20T13:12:47Z

Coverage remained the same at 96.359% when pulling 009fe53 on lucasfcosta:document-new-array-matchers into 1924a89 on sinonjs:master.

fearphage · 2016-12-20T15:07:10Z

docs/_releases/v2.0.0-pre.4/matchers.md

@@ -112,6 +112,26 @@ Requires the value to be a `Function`.
 Requires the value to be an `Array`.


+#### `sinon.match.array.deepEquals(iterable)`
+
+Requires an iterable value to be deep equal another one.


Once you see it written down, this is weird. You're checking inalterable from the array matcher. I would expect the array namespace to contain array matchers.

@fearphage I agree, I also raised the same question in a comment at my PR at #1208.

However, I was just thinking if we should act upon those iterables as if they were arrays, after all this matcher has the word array in it.

Since we're calling methods from Array.prototype and passing a value to be used as this it would work if users had their own array implementations or with things like arguments.

I can change this if you want me too, I'd like to do whatever you prefer. If we want to be more strict it would be good to accept Arrays only, but if we want to allow other similar implementations this is the way to go.

Maybe we could also have an match.iterable or something like that.

What do you think it's better? Let me know and we can fix this if needed 😄

Also, sorry for not having raised that question more actively before. 😓

I understand the benefits and usage. I'm just saying when you document it, it looks weird/out of place. I would expect an array matcher to throw/error if I didn't pass it an array honestly. I was considering an iterable matcher. I'd wait to see what other think about it though.

@fearphage Yes, a new iterable matcher would be the most adequate option IMO.

Thanks for your input, I totally agree with everything you said.

Changing array to iterable is fine with me, as long as we do it now, before a new release.

But do we have a definition of iterable? Objects are iterable in a sense too (for ... in) ... Is it anything that has a length property?

expanding the namespace through match.map., match.set., etc

This is going to be a lot of duplicate or (hopefully) resused code, but I think that is the most explicit option.

Agree/disagree, @fearphage?

So the first option being to make this only work for arrays since it's in the array namespace? If that is the case, I would agree. I think we should confirm that though.

For instance, matcher.array.startsWith(...) should throw when you don't pass an array to it. It seems like you're doing 2 things at once here. matcher.array() && array.startsWith(). So if I pass {length: 1, 0: 'foo'}, it would fail saying the test expected an array. Are we in agreement or did I miss the mark?

So no duck typing then? Strictly Array.isArray()? I am not super sure what it really brings to the table, since it would leave out arguments and other array-like objects, but people can always convert these to proper arrays if need be, so it should be ok.

And since the test is for an array, I guess it should break on non-arrays.

@fatso83 @fearphage I totally agree with that. People using this matcher should be aware it will work for Array objects only and anything else should be converted to that.

Ok, seems all are pro. Care to update the docs, Lucas? As long as you don't change the interface you document you can also add any strengthening assertions you like.

Of course!
I'm already working on the Map and Set matchers, for now I'll make sure to add commits allowing arrays only on the matchers we already have and update the docs. 😄

fatso83 · 2016-12-23T16:00:24Z

Btw, the docs have been put in the wrong folder. docs/_releases is for docs on previous, released versions. docs/release-source is for what is will be released.

coveralls · 2016-12-25T20:46:15Z

Coverage increased (+0.004%) to 96.37% when pulling 739e4b0 on lucasfcosta:document-new-array-matchers into 2df140f on sinonjs:master.

coveralls · 2016-12-25T20:48:23Z

Coverage increased (+0.004%) to 96.37% when pulling a100d08 on lucasfcosta:document-new-array-matchers into 2df140f on sinonjs:master.

lucasfcosta · 2016-12-25T20:50:10Z

Here it is 😄
These matchers will now fail whenever actual is not an Array and I also updated the docs (commited w/ --amend).
Let me know if I forgot something.

I have already started working on matchers for the Map and Set objects and I'll submit another PR for that later this week.

PS.: Sorry for the coveralls spam, I was making sure the commit log was clean and so it ended up doing many builds.

coveralls · 2016-12-25T20:53:04Z

Coverage increased (+0.004%) to 96.37% when pulling a8064c0 on lucasfcosta:document-new-array-matchers into 2df140f on sinonjs:master.

coveralls · 2016-12-25T20:54:05Z

Coverage increased (+0.004%) to 96.37% when pulling a8064c0 on lucasfcosta:document-new-array-matchers into 2df140f on sinonjs:master.

coveralls · 2016-12-25T20:55:15Z

Coverage increased (+0.004%) to 96.37% when pulling cd1e396 on lucasfcosta:document-new-array-matchers into 2df140f on sinonjs:master.

fatso83 · 2016-12-26T09:54:01Z

Great work. Thanks for the collaboration :-)

fearphage reviewed Dec 20, 2016

View reviewed changes

fatso83 mentioned this pull request Dec 23, 2016

Add .resolves and .rejects to stub #1211

Merged

lucasfcosta force-pushed the document-new-array-matchers branch from 009fe53 to d794443 Compare December 25, 2016 20:44

lucasfcosta added 4 commits December 25, 2016 18:44

Array deepEquals matcher fails when actual is not an array

a1e7f68

Array startsWith matcher fails when actual is not an array

3c9d6cf

Array endsWith matcher fails when actual is not an array

bec0b4f

Array contains matcher fails when actual is not an array

bd9d57f

lucasfcosta force-pushed the document-new-array-matchers branch from d794443 to 739e4b0 Compare December 25, 2016 20:44

lucasfcosta force-pushed the document-new-array-matchers branch from 739e4b0 to a100d08 Compare December 25, 2016 20:47

lucasfcosta force-pushed the document-new-array-matchers branch 2 times, most recently from f9cacca to a8064c0 Compare December 25, 2016 20:52

Add docs for new array matchers

cd1e396

lucasfcosta force-pushed the document-new-array-matchers branch from a8064c0 to cd1e396 Compare December 25, 2016 20:53

fatso83 merged commit c446844 into sinonjs:master Dec 26, 2016

lucasfcosta mentioned this pull request Dec 27, 2016

Maps matchers #1215

Merged

lucasfcosta mentioned this pull request Jan 4, 2017

Sets matchers #1232

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docs for new array matchers #1210

Add docs for new array matchers #1210

lucasfcosta commented Dec 20, 2016

coveralls commented Dec 20, 2016 •

edited

Loading

fearphage Dec 20, 2016

lucasfcosta Dec 20, 2016 •

edited

Loading

fearphage Dec 20, 2016 •

edited

Loading

lucasfcosta Dec 20, 2016

fatso83 Dec 20, 2016

fearphage Dec 24, 2016

fatso83 Dec 25, 2016

lucasfcosta Dec 25, 2016

fatso83 Dec 25, 2016

lucasfcosta Dec 25, 2016 •

edited

Loading

fatso83 commented Dec 23, 2016

coveralls commented Dec 25, 2016 •

edited

Loading

coveralls commented Dec 25, 2016 •

edited

Loading

lucasfcosta commented Dec 25, 2016 •

edited

Loading

coveralls commented Dec 25, 2016 •

edited

Loading

coveralls commented Dec 25, 2016 •

edited

Loading

coveralls commented Dec 25, 2016 •

edited

Loading

fatso83 commented Dec 26, 2016

Add docs for new array matchers #1210

Add docs for new array matchers #1210

Conversation

lucasfcosta commented Dec 20, 2016

Purpose (TL;DR)

Solution

How to verify

coveralls commented Dec 20, 2016 • edited Loading

fearphage Dec 20, 2016

Choose a reason for hiding this comment

lucasfcosta Dec 20, 2016 • edited Loading

Choose a reason for hiding this comment

fearphage Dec 20, 2016 • edited Loading

Choose a reason for hiding this comment

lucasfcosta Dec 20, 2016

Choose a reason for hiding this comment

fatso83 Dec 20, 2016

Choose a reason for hiding this comment

fearphage Dec 24, 2016

Choose a reason for hiding this comment

fatso83 Dec 25, 2016

Choose a reason for hiding this comment

lucasfcosta Dec 25, 2016

Choose a reason for hiding this comment

fatso83 Dec 25, 2016

Choose a reason for hiding this comment

lucasfcosta Dec 25, 2016 • edited Loading

Choose a reason for hiding this comment

fatso83 commented Dec 23, 2016

coveralls commented Dec 25, 2016 • edited Loading

coveralls commented Dec 25, 2016 • edited Loading

lucasfcosta commented Dec 25, 2016 • edited Loading

coveralls commented Dec 25, 2016 • edited Loading

coveralls commented Dec 25, 2016 • edited Loading

coveralls commented Dec 25, 2016 • edited Loading

fatso83 commented Dec 26, 2016

coveralls commented Dec 20, 2016 •

edited

Loading

lucasfcosta Dec 20, 2016 •

edited

Loading

fearphage Dec 20, 2016 •

edited

Loading

lucasfcosta Dec 25, 2016 •

edited

Loading

coveralls commented Dec 25, 2016 •

edited

Loading

coveralls commented Dec 25, 2016 •

edited

Loading

lucasfcosta commented Dec 25, 2016 •

edited

Loading

coveralls commented Dec 25, 2016 •

edited

Loading

coveralls commented Dec 25, 2016 •

edited

Loading

coveralls commented Dec 25, 2016 •

edited

Loading