require-expect with "except-simple" config shows too many errors about eslint-plugin-qunit HOT 6 CLOSED

Just to make sure I understand correctly:

QUnit.test("Missing expect, should warn once", function (assert) {
    assert.ok(true, "This will warn");
    assert.ok(true, "This also will warn");
});

I think the "always" mode warns on the QUnit.test() CallExpression, maybe we should also warn on the same CallExpression in the "except-simple" strategy? It could be argued that it's really a test-level issue if an expect() declaration is missing... What do you think?

from eslint-plugin-qunit.

@platinumazure Actually I've recently run with the "except-simple" strategy and it also warns on the QUnit.test() line.

The issue, per the code you've shown above, is that you'll get two errors, both pointing to the first line.

from eslint-plugin-qunit.

First thought on implementation is to add a didReport flag to the currentTest object. Then we can check for that flag here: https://github.com/platinumazure/eslint-plugin-qunit/blob/master/lib/rules/require-expect.js#L87

from eslint-plugin-qunit.

@mitchlloyd That sounds pretty good.

I just want to make sure, are we basically in agreement that always and except-simple should both report once on the QUnit.test() CallExpression? If we agree on that point, you can consider this accepted. If you want to write a PR, let me know, otherwise hopefully I can get to it later this week. Thanks!

from eslint-plugin-qunit.

@platinumazure We are in agreement that one error on the QUnit.test() CallExpression is the right way to go. The fix needed (adding expect(n)) is a general test() concern.

I should have time to get to this in the next couple of days and will get a PR together.

from eslint-plugin-qunit.

Sounds good, let me know if you find yourself too busy, otherwise I look forward to a PR.

from eslint-plugin-qunit.