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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with having a single unit test #10

Closed
vieiralucas opened this issue Oct 12, 2016 · 1 comment 路 Fixed by #14
Closed

Problem with having a single unit test #10

vieiralucas opened this issue Oct 12, 2016 · 1 comment 路 Fixed by #14

Comments

@vieiralucas
Copy link
Member

Currently unit tests for get-func-name are so tiny and simple that I can just copy and paste it over here 馃槃

'use strict';
var assert = require('simple-assert');
var getFuncName = require('..');
describe('getFuncName', function () {
  it('getFuncName', function () {
    // Asserting that `getFuncName` behaves correctly
    function /*one*/correctName/*two*/() { // eslint-disable-line no-inline-comments, spaced-comment
      return 0;
    }
    function withoutComments() {
      return 1;
    }

    var anonymousFunc = (function () {
      return function () { // eslint-disable-line func-style
        return 2;
      };
    }());
    assert(getFuncName(correctName) === 'correctName');
    assert(getFuncName(withoutComments) === 'withoutComments');
    assert(getFuncName(anonymousFunc) === '');
  });
});

I know this is beautiful, but...

  • If the test fails, we wont be able to tell what's going on just by reading the message.
    screen shot 2016-10-11 at 9 20 48 pm
    What have I broke here? Imagine if this fails only in IE running over saucelabs 馃樃.
  • If this keeps growing I'm afraid that this can become very confusing like the utilities tests of chaijs/chai.

As @meeber suggested here, I think that we should refactor this test and split it so it has a single assertion per test and better names.
I understand that get-func-name should not grow a lot and will probably stay very simple, but I don't see this as a good reason to not have good descriptive tests.

If you guys agree. I'll be very happy to make a PR addressing this.

@lucasfcosta
Copy link
Member

I totally agree.
That would be great.

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 a pull request may close this issue.

2 participants