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

td.reset() it´s working has expected? #133

Closed
bySabi opened this issue Sep 18, 2016 · 5 comments
Closed

td.reset() it´s working has expected? #133

bySabi opened this issue Sep 18, 2016 · 5 comments

Comments

@bySabi
Copy link

bySabi commented Sep 18, 2016

I had been testing some React components with enzyme and tape.

I testdouble console.error function in order of capture React console Warning: and my custom generated Warning: too:

Ex:

  t.test('required "to" prop', t => {
    let wrapper;
    // spy 'console.error' called by Facebook´s warning
    const consoleError = console.error;   /* add cause td.reset() not reset spy */
    console.error = td.function('console.error');
    t.test('setup', t => {
      wrapper = mount(<Scrollchor className="test-chor">Test</Scrollchor>);
      t.end();
    });

    t.test('required "to" prop', t => {
      const explain = td.explain(console.error);
      t.equal(explain.callCount, 1);
      const error = explain.calls[0].args[0];
      t.ok(!!~error.indexOf('Warning: Failed prop type: Required prop `to`')); // search 'substring'
      t.end();
    });

    t.test('tearDown', t => {
      wrapper.unmount();
      td.reset();
      console.error = consoleError;   /* add cause td.reset() not reset spy */
      t.end();
    });
    t.end();
  });

console.error "spying?" works very good but on next unit test console.error is still mocked. I suppose that td.reset() will get it back but is not happend. That´s was I add consoleError, see /* .. */ comments.

td.reset() is working right?

Thanks for the hard work on testdouble is very easy to use.

@searls
Copy link
Member

searls commented Sep 18, 2016

td.reset can't know what you're overriding unless you use td.replace to allow it the ability to reassign it. Therefore, what you have:

console.error = td.function('console.error');

will not be resettable because td.js can't know what you assigned that td to. However, this should work:

td.replace(console, 'error')

In any case you should consider wrapping the console with a function you do own. See this article for more details: https://github.com/testdouble/contributing-tests/wiki/Don%27t-mock-what-you-don%27t-own

@searls searls closed this as completed Sep 18, 2016
@bySabi
Copy link
Author

bySabi commented Sep 18, 2016

@searls td.replace and td.reset works very well, thanks.

One more thing. Is any problem with this console.error "Warnings" spy approach?. I read that wiki again to day but still not figure out how and why wrapping console with my own function could be helpme. Any advice?

@searls
Copy link
Member

searls commented Sep 19, 2016

In short, because the purpose of test double libraries is not to "fake stuff out so that hard-to-test things become testable". The purpose of test double libraries is to help you discover that hard-to-test things could be better-designed.

In this case, your code depends on a built-in host function that you don't control (console.error), which means you can't introspect/wrap/tap into it. You can indeed just fake it out like you did in this case, but what if the console.error was hard to mock or replace? (Indeed, this is a good example, because many hosts like MSIE don't take kindly to reassigning the console functions.) Attempting to deal with hard-to-use or hard-to-mock APIs by way of increasingly clever test double strategies is a useless outlet for the pain.

Instead, test double aficionados like myself will encourage you to wrap all third party libraries you use where you'd want to mock them out. So, in this case, I would write my own module like this:

module.exports = {
  error: console.error
}

And maybe name it wrap/console or adapter/log or something and then replace that module with td.replace. That way if I ever need to change that API or its implementation, perhaps due to some pain in testing it, I'm already well-positioned to make those changes.

Test doubles and outside-in testing is a highly-opinionated commentary on how we design our code and separate the code we own from the code we don't. Very few people really think very hard about this in practice, because the universe of people who care about nuanced software design is tiny and the universe of people who just need a library to fake stuff out is massive.

@bySabi
Copy link
Author

bySabi commented Sep 19, 2016

The purpose of test double libraries is to help you discover that hard-to-test
things could be better-designed.

I ended removing all console.error based test :-). And looking for better checks. React throw 'Warnings' only on dev mode. In production console.error base test will fail.

Very few people really think very hard about this in practice, 
because the universe of people who care about nuanced software design
is tiny and the universe of people who just need a library to fake stuff out is massive.

I´m a unit test newbie but choose learn and use: tape/testdouble for React test beside massive choosed jasmine/sinon cause I can see from where test code come and I feel more in-control and less "bewitched".

Thanks for In short, ... words :-). Your advice was really useful.

I have working test with React, Enzyme, Tape and TestDouble here: https://github.com/bySabi/react-scrollaware/blob/master/test/scrollaware-test.js ... any suggestion from you is really welcome. Only If you have the time to take a look, of course.

@searls
Copy link
Member

searls commented Sep 19, 2016

Thanks very much for the kind words bySabi. If you get the time to read more of our testing wiki or about test double's motivations, please let me know if they help.

Feel free to open more questions if you have them.

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