Implement new rule no-assert-ok

[ventuno]

@platinumazure here's my attempt at implementing #77.

Apologies for this not being super polished, but I wanted to have something out to make sure we were on the same page.
A few notes:

1. The general idea behind this is to provide a function (createAssertionCheck -- still thinking of a better name) that can look for usages of assertions (global or against the local assert object);
2. For example, based on the above no-assert-equal could for example be refactored as:

// metadata ...
meta: {
    // ...
    messages: {
        unexpectedGlobalEqual: "Unexpected equal. Use strictEqual, deepEqual, or propEqual.",
        unexpectedAssertEqual: "Unexpected {{assertVar}}.equal. Use {{assertVar}}.strictEqual, {{assertVar}}.deepEqual, or {{assertVar}}.propEqual."
    },
    // ...
},
create: utils.createAssertionCheck(["equal"], "unexpectedGlobalEqual", "unexpectedAssertEqual")

3. Similarly no-loose-assertions could be implemented as:

// metadata ...
meta: {
    // ...
    messages: {
        unexpectedGlobalLooseAssertion: "Unexpected {{assertion}}. Use ...",
        unexpectedLocalLooseAssertion: "Unexpected {{assertVar}}.{{assertion}}. Use ..."
    },
    // ...
},
create: function(context) {
    const defaultOptions = ["equal", "ok", "notOk"];
    // Allow users to configure which assertions they want to use.
    const options = overrideDefaults(context.options, defaultOptions);
    return utils.createAssertionCheck(options, "unexpectedGlobalLooseAssertion", "unexpectedLocalLooseAssertion").call(this, context);
}

4. Finally utils may not be the best place for createAssertionCheck maybe it could live in its own utility file?

TODO:

• docs;
• export new rule.

[coveralls]

Coverage remained the same at 100.0% when pulling 4d5f1a8 on ventuno:ftr-77 into b3e46ec on platinumazure:master.

[platinumazure]

Thanks for the PR! I'll hopefully have time to review this weekend.

[ventuno]

Thanks! Take your time, I'll be traveling next week so I might have a little less time to implement the feedback, but I'll definitely keep an eye on this :-).

[platinumazure]

Hi, just wanted to say I haven't forgotten about this but I've been very busy. Unfortunately, this weekend will be another busy one, but I'm hoping to look at this in the next couple of weeks.

[ventuno]

No problema. Thanks for the update!

[platinumazure]

Hi @ventuno, thanks for putting this together and thank you as well for your patience as I was delayed in reviewing this.

I've called out a few minor issues via inline comments.

Here's a few more things:

1. Could you please add some tests for the new code in lib/utils.js? One option might be to create a fake/test rule that uses the function (maybe with fake assertion names, etc.) and then run RuleTester on that rule to make sure it demonstrates the expected behavior.
2. Please look at the Travis CI results-- there are some failing tests. Most of these have to do with ensuring the rule is referenced in the README, as well as ensuring the rule is available in the plugin definition file (index.js in the project's top-level directory). Can you please fix these?

Thanks again for contributing, I appreciate it!

[ventuno]

Thanks for the review @platinumazure! I addressed all comments can you please take another look?

1. Could you please add some tests for the new code in `lib/utils.js`? One option might be to create a fake/test rule that uses the function (maybe with fake assertion names, etc.) and then run RuleTester on that rule to make sure it demonstrates the expected behavior.

I can definitely do that, but I'm not 100% if such test would be really valuable? After all this is implicitly tested once the code is used by no-assert-ok. If you feel strongly a fake test rule would help I can definitely do that.

2. Please look at the Travis CI results-- there are some failing tests. Most of these have to do with ensuring the rule is referenced in the README, as well as ensuring the rule is available in the plugin definition file (`index.js` in the project's top-level directory). Can you please fix these?

Done!

[platinumazure]

Thank you for addressing my feedback!

Only one request: Could you please add some correct code examples to the documentation? Probably that just means some uses of the other assertion types that could be used instead.

Otherwise looks great! Thanks again for contributing!

[platinumazure]

And I'm okay with skipping tests on the utility method at this point. If I change my mind later, I can always figure out the tests myself.

[ventuno]

Thanks @platinumazure, I updated the docs. Please take another look.

Also, should we discuss next steps? With reference to the description:

1. should I put up a PR to refactor no-assert-equal to use the new utils.createAssertionCheck?
2. should we discuss again no-loose-assertions as a new rule to replace no-assert-ok and no-assert-equal altogether?

[ventuno]

@platinumazure any update on this?

[ventuno]

@platinumazure wondering if there's anything left for me to do on this? I addressed your latest request for changes and added some correct code examples to the docs.

I'm looking forward to starting on the next steps!

[platinumazure]

Sorry, everything is good on your end (I'm pretty sure), I've just been busy.

My suggestion is, let's not waste time refactoring the other rule because it would also be made obsolete by no-loose-assertions.

Next steps:

1. I'll do a final check and merge this if all looks good
2. At any time, you could start working on no-loose-assertions if you want, but there's no rush
3. I need to fix my npm publish issues
4. I'll cut a last minor release for this major release line with this rule in it
5. When no-loose-assertions is ready, it gets merged in and I mark the other rules as deprecated
6. I'll cut a new major release

What do you think?

[platinumazure]

This looks ready to merge.

Just one thing: You didn't name yourself as @author in lib/rules/no-assert-ok.js. This isn't required and I would be okay with merging as-is, but you did write an author name in tests/lib/rules/no-assert-ok.js. So it's up to you.

If you want to make that change, feel free to just push an extra commit to this branch. I'll squash the commits during merge. Otherwise, if you're okay with me committing as-is, just let me know and I'll merge.

Thanks for contributing, and thanks also for your extraordinary patience. :-)

[ventuno]

Thanks @platinumazure, I just pushed a new change with the @author annotation. Thanks for pointing that out.

Also, thanks again for your support. I'm glad we did this. I think it's a step in better testing for all of us! I'll start working on no-loose-assertions rule as soon as this is merged.

[platinumazure]

@ventuno Just as a follow-up, I finally regained access to my npm account today. Hoping to publish a minor release with this PR soon (maybe this weekend).

[ventuno]

That's great news. Thanks @platinumazure!

[ventuno]

@platinumazure just enabled this today. Thanks a lot for the release :-).