"no-commented-test" false positive

[Turbo87]

// TODO: refactor with a Component test (instead of an Acceptance test)

led to triggering the no-commented-test rule. we should probably check for test( instead of just test.

[platinumazure]

Thanks for the issue! Agreed that this is a false positive and a bug.

Unfortunately, the regular expression does check test\s*\( (among other things). Technically, a space can be used between a call expression's callee and the argument parentheses. But now I'll have to come up with something better, maybe involving a parser.

[platinumazure]

Hi @Turbo87, sorry for taking so long to get to this.

I have a branch up that might be a pretty good approach (without relying on trying to do a fuzzy parse).

Basically it will only warn if the characters after test( (etc.) meet one of the following criteria:

• Could be a valid string (i.e., has matching single quotes/double quotes/backticks with content in between, i.e., potential test name)
• Could be multiple args (contiguous non-space/-right-paren/-comma text, followed by comma with optional space on either side, followed by more text-- multiple words with space and no comma are automatically allowed)

So there will still be a few false positives, but much, much fewer than before... Looking forward to any feedback you might have.

[Krinkle]

Ran into another one in QUnit's own code base:

// Run actual test (without context).

Perhaps to improve the signal-noise ratio we could reduce the pattern to only cases where a quote follows the open parenthesis? E.g. extend the pattern by \s*['"]. This in theory could miss a case where a variable name is used, but I think that's perhaps sufficiently rare to be worth the small loss in assistance.

[platinumazure]

@Krinkle I like that idea but still want to give users the option either way. How about if we create a new option, defaulting to the current behavior but letting users opt in to the more lenient check? And we can flip the default option value on the next major release.

[platinumazure]

I've decided that we can just do the quote check as a non-breaking change (as it will result in fewer warnings).

I'll work on a PR.