Fake docs improvement #2360

srknzl · 2021-04-23T13:58:02Z

Purpose (TL;DR)

Tries to improve documentation for the fake API. Resolves #2070

Todo

I went over this comment, that has these bullet points:

Make sure the list of static factory methods on sinon.fake is complete

yes, the list is complete except for usingPromise

Convert and/or add some runnable Runkit examples (very easy, but takes a bit of time)

I'm on it, I wanted open this pr so that you can check out current status.

Update the link in Instance properties to point to the same section in Spies.

Only firstArg was missing a reference, added that. Changed others' links so that they are linked to the exact same header.

State that the listed props are just one of the many from the spies documentation and that you can refer to that for the complete list
Clarify the wording so that it becomes clear that 1. The Fakes API is an alternative to the Stubs and Spies API that can fully replace all such uses. 2. It is intended to be simpler to use, maintain and has a lot smaller API surface and avoids many bugs by having quazi-immutable Fake instances (the behavior cannot be changed after creation (unlike Stubs), but they do have internal state that changes when used, like the callCount and other stats) 3. A Fake instance has the API of a Spy (

I've done my best to make it clear.

How to verify

Check out this branch
npm install
cd docs
gem install bundler
bundle install
bundle exec jekyll serve
Check the website

Checklist for author

npm run lint passes
References to standard library functions are cached.

My Questions

Which versions should change? I changed v10.0.1 for now since I am not sure.

srknzl · 2021-04-23T14:01:22Z

docs/_releases/v10.0.1/fakes.md

+### When to use fakes?
+
+Fakes are alternatives to the Stubs and Spies, and they can fully replace all such use cases.
+
+They are intended to be simpler to use, and avoids many bugs by having immutable behaviour.
+
+The created `fake` `Function`, with or without behavior has the same API as a (`sinon.spy`)[spies].
+


I thought adding this new lines to introduction would be too much for an introduction section. So moved one line from there to here.

srknzl · 2021-04-23T14:02:03Z

docs/_releases/v10.0.1/fakes.md

+#### Using fakes instead of spies
+
+```js
+var foo = {
+  bar: () => {
+    return 'baz';
+  }
+};
+// wrap existing method without changing its behaviour
+var fake = sinon.replace(foo, 'bar', sinon.fake(foo.bar));
+
+fake();
+// baz
+
+fake.callCount;
+// 1
+```
+
+#### Using fakes instead of stubs
+
+```js
+var foo = {
+  bar: () => {
+    return 'baz';
+  }
+};
+// replace method with a fake one
+var fake = sinon.replace(foo, 'bar', sinon.fake.returns('fake value'));
+
+fake();
+// fake value
+
+fake.callCount;
+// 1
+```
+


I think this alternative use cases helps as I stated in the issue #2070

fatso83 · 2021-04-26T09:27:26Z

This is a great start, but the changes are in the wrong folder :) See https://github.com/sinonjs/sinon/blob/master/docs/CONTRIBUTING.md#documenting-the-next-release

srknzl · 2021-04-26T19:20:44Z

I moved the changes to latest docs. I wonder how I can run examples in the repo. I am asking because when I do the following I get an error:

cd docs/release-source/release/examples
npm i
npm test

The error

Oops! Something went wrong! :(

ESLint: 6.8.0.

ESLint couldn't find the plugin "eslint-plugin-local-rules".

(The package "eslint-plugin-local-rules" was not found when loaded as a Node module from the directory "/home/serkan/forked/sinon/docs/release-source/release/examples".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

    npm install eslint-plugin-local-rules@latest --save-dev

The plugin "eslint-plugin-local-rules" was referenced from the config file in ".eslintrc.yml".

If you still can't figure out the problem, please stop by https://gitter.im/eslint/eslint to chat with the team.

internal/modules/cjs/loader.js:960
  throw err;
  ^

Error: Cannot find module 'supports-color'
Require stack:
- /home/serkan/forked/sinon/lib/sinon/color.js
- /home/serkan/forked/sinon/lib/sinon/spy-formatters.js
- /home/serkan/forked/sinon/lib/sinon/proxy.js
- /home/serkan/forked/sinon/lib/sinon/stub.js
- /home/serkan/forked/sinon/lib/sinon/mock-expectation.js
- /home/serkan/forked/sinon/lib/sinon/mock.js
- /home/serkan/forked/sinon/lib/sinon/sandbox.js
- /home/serkan/forked/sinon/lib/sinon/create-sandbox.js
- /home/serkan/forked/sinon/lib/sinon.js
- /home/serkan/forked/sinon/docs/release-source/release/examples/spies-1-pubsub.test.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:957:15)
    at Function.Module._load (internal/modules/cjs/loader.js:840:27)
    at Module.require (internal/modules/cjs/loader.js:1019:19)
    at require (internal/modules/cjs/helpers.js:77:18)
    at Object.<anonymous> (/home/serkan/forked/sinon/lib/sinon/color.js:3:21)
    at Module._compile (internal/modules/cjs/loader.js:1133:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1153:10)
    at Module.load (internal/modules/cjs/loader.js:977:32)
    at Function.Module._load (internal/modules/cjs/loader.js:877:14)
    at Module.require (internal/modules/cjs/loader.js:1019:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/serkan/forked/sinon/lib/sinon/color.js',
    '/home/serkan/forked/sinon/lib/sinon/spy-formatters.js',
    '/home/serkan/forked/sinon/lib/sinon/proxy.js',
    '/home/serkan/forked/sinon/lib/sinon/stub.js',
    '/home/serkan/forked/sinon/lib/sinon/mock-expectation.js',
    '/home/serkan/forked/sinon/lib/sinon/mock.js',
    '/home/serkan/forked/sinon/lib/sinon/sandbox.js',
    '/home/serkan/forked/sinon/lib/sinon/create-sandbox.js',
    '/home/serkan/forked/sinon/lib/sinon.js',
    '/home/serkan/forked/sinon/docs/release-source/release/examples/spies-1-pubsub.test.js'
  ]
}

When I install npm install eslint-plugin-local-rules@latest --save-dev, and run npm test again another one occurs:

The other error

Error: Failed to load plugin 'local-rules' declared in '.eslintrc.yml': eslint-plugin-local-rules: Cannot find "eslint-local-rules.{,.cjs}(looking up from "/home/serkan/forked/sinon/docs/release-source/release/examples/node_modules/eslint-plugin-local-rules").
    at Object.<anonymous> (/home/serkan/forked/sinon/docs/release-source/release/examples/node_modules/eslint-plugin-local-rules/index.js:12:9)
    at Module._compile (/home/serkan/forked/sinon/docs/release-source/release/examples/node_modules/v8-compile-cache/v8-compile-cache.js:192:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1153:10)
    at Module.load (internal/modules/cjs/loader.js:977:32)
    at Function.Module._load (internal/modules/cjs/loader.js:877:14)
    at Module.require (internal/modules/cjs/loader.js:1019:19)
    at require (/home/serkan/forked/sinon/docs/release-source/release/examples/node_modules/v8-compile-cache/v8-compile-cache.js:159:20)
    at ConfigArrayFactory._loadPlugin (/home/serkan/forked/sinon/docs/release-source/release/examples/node_modules/eslint/lib/cli-engine/config-array-factory.js:978:42)
    at /home/serkan/forked/sinon/docs/release-source/release/examples/node_modules/eslint/lib/cli-engine/config-array-factory.js:848:33
    at Array.reduce (<anonymous>)
internal/modules/cjs/loader.js:960
  throw err;
  ^

Error: Cannot find module 'supports-color'
Require stack:
- /home/serkan/forked/sinon/lib/sinon/color.js
- /home/serkan/forked/sinon/lib/sinon/spy-formatters.js
- /home/serkan/forked/sinon/lib/sinon/proxy.js
- /home/serkan/forked/sinon/lib/sinon/stub.js
- /home/serkan/forked/sinon/lib/sinon/mock-expectation.js
- /home/serkan/forked/sinon/lib/sinon/mock.js
- /home/serkan/forked/sinon/lib/sinon/sandbox.js
- /home/serkan/forked/sinon/lib/sinon/create-sandbox.js
- /home/serkan/forked/sinon/lib/sinon.js
- /home/serkan/forked/sinon/docs/release-source/release/examples/spies-1-pubsub.test.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:957:15)
    at Function.Module._load (internal/modules/cjs/loader.js:840:27)
    at Module.require (internal/modules/cjs/loader.js:1019:19)
    at require (internal/modules/cjs/helpers.js:77:18)
    at Object.<anonymous> (/home/serkan/forked/sinon/lib/sinon/color.js:3:21)
    at Module._compile (internal/modules/cjs/loader.js:1133:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1153:10)
    at Module.load (internal/modules/cjs/loader.js:977:32)
    at Function.Module._load (internal/modules/cjs/loader.js:877:14)
    at Module.require (internal/modules/cjs/loader.js:1019:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/serkan/forked/sinon/lib/sinon/color.js',
    '/home/serkan/forked/sinon/lib/sinon/spy-formatters.js',
    '/home/serkan/forked/sinon/lib/sinon/proxy.js',
    '/home/serkan/forked/sinon/lib/sinon/stub.js',
    '/home/serkan/forked/sinon/lib/sinon/mock-expectation.js',
    '/home/serkan/forked/sinon/lib/sinon/mock.js',
    '/home/serkan/forked/sinon/lib/sinon/sandbox.js',
    '/home/serkan/forked/sinon/lib/sinon/create-sandbox.js',
    '/home/serkan/forked/sinon/lib/sinon.js',
    '/home/serkan/forked/sinon/docs/release-source/release/examples/spies-1-pubsub.test.js'
  ]
}

Installing supports-color does not help here, same error happens if I do that.

fatso83 · 2021-04-26T21:55:28Z

@srknzl You touch upon something that I was fiddling with just a few weeks back, but I was surprised to see that I have not committed any of those changes. I don't remember quite what I was doing even ...

You can run the tests from the root easily:

npm run test-runnable-examples

That again only runs the same things you just did (from the examples folder), which runs the tests and does some linting:

docs/release-source/release/examples/run-test.sh

I did not get exactly the same errors as you did, but I think I fixed the issues up now. Try pulling and rebasing on top of master. In any case, the errors were just about the linting (no idea what the local rules thing was about), and the tests ran fine on my machine at least, even if eslint complained.

…ment

srknzl · 2021-05-02T08:33:47Z

I am trying to create fake examples with existing spy and stub examples. Is it a good approach? I can also add additional examples for methods that fake has but spy/stub does not have; what are they?

I finished spy examples. There are two interesting things:

sinon.fake cannot wrap all the methods of an object, so I omitted that test.
A fake instance does not have withArgs method, instead of it I used calledWith()

How many examples we will include for fake docs? Where we will put them? Any plans? I think it is better to plan this first.

Am I supposed to write runkit examples only for existing snippets in the fake page?

fatso83

How many examples we will include for fake docs? Where we will put them? Any plans? I think it is better to plan this first.
I think the way you did it looks just fine! But AFAIK, they are just in the examples source directory and they would also need to be included on the page for them to show, so that means editing the markdown as done for the other types.

Am I supposed to write runkit examples only for existing snippets in the fake page?

Yes. Not sure what else to do? Sinon is a collaborative effort, so if you think that something is missing, then feel free to add the changes you would like to see.

docs/release-source/release/fakes.md

…documentation

srknzl · 2021-05-08T19:06:53Z

Converted all existing snippets to runkit + addressed your review points @fatso83 . I am open to another review at this point

fatso83 · 2021-05-25T07:56:44Z

Thank you for all your efforts! Exemplary PR <3

srknzl · 2021-05-31T07:14:54Z

I realized I did not used runkit snippets I wrote. Why did not you warn me :) Should I fix it in a separate pr? @fatso83

fatso83 · 2021-05-31T07:59:33Z

If you could, that would be great, @srknzl. Sorry, I did not check whether they were included in the markdown :(

srknzl added 3 commits April 23, 2021 16:45

Clarify fake api, add missing references to spy api

af5c695

Mention return value in sandbox.replace()

3687424

Merge remote-tracking branch 'upstream/master'

224c8e8

srknzl mentioned this pull request Apr 23, 2021

Improve docs for fakes #2070

Closed

srknzl commented Apr 23, 2021

View reviewed changes

srknzl marked this pull request as ready for review April 23, 2021 14:08

srknzl added 2 commits April 26, 2021 22:04

Move fake changes to latest docs

8b2b89c

Move the sandbox change to latest docs

f6eb7f6

srknzl added 2 commits May 2, 2021 10:38

Merge remote-tracking branch 'upstream/master' into docs-fake-improve…

cbdea28

…ment

Add fake runkit examples using existing spy examples

232064e

Fix pubsub fake example

61a4305

fatso83 reviewed May 3, 2021

View reviewed changes

docs/release-source/release/fakes.md Outdated Show resolved Hide resolved

docs/release-source/release/fakes.md Outdated Show resolved Hide resolved

srknzl added 2 commits May 8, 2021 18:03

Change runnable examples to be the same with the ones in the current …

7b3895c

…documentation

Add examples, address some review comments

a8194a1

srknzl requested a review from fatso83 May 8, 2021 19:08

fatso83 merged commit 40f5ca3 into sinonjs:master May 25, 2021

srknzl mentioned this pull request Jun 1, 2021

Use runkit snippets in fake documentation #2381

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fake docs improvement #2360

Fake docs improvement #2360

srknzl commented Apr 23, 2021 •

edited

srknzl Apr 23, 2021

srknzl Apr 23, 2021

fatso83 commented Apr 26, 2021

srknzl commented Apr 26, 2021

fatso83 commented Apr 26, 2021 •

edited

srknzl commented May 2, 2021 •

edited

fatso83 left a comment

srknzl commented May 8, 2021 •

edited

fatso83 commented May 25, 2021

srknzl commented May 31, 2021

fatso83 commented May 31, 2021

Fake docs improvement #2360

Fake docs improvement #2360

Conversation

srknzl commented Apr 23, 2021 • edited

Purpose (TL;DR)

Todo

How to verify

Checklist for author

My Questions

srknzl Apr 23, 2021

Choose a reason for hiding this comment

srknzl Apr 23, 2021

Choose a reason for hiding this comment

fatso83 commented Apr 26, 2021

srknzl commented Apr 26, 2021

fatso83 commented Apr 26, 2021 • edited

srknzl commented May 2, 2021 • edited

fatso83 left a comment

Choose a reason for hiding this comment

srknzl commented May 8, 2021 • edited

fatso83 commented May 25, 2021

srknzl commented May 31, 2021

fatso83 commented May 31, 2021

srknzl commented Apr 23, 2021 •

edited

fatso83 commented Apr 26, 2021 •

edited

srknzl commented May 2, 2021 •

edited

srknzl commented May 8, 2021 •

edited