qunit/require-expect reports some warnings that it should not

[cah-brian-gantzler]

The following report a missing expect when an expect should not be needed.

This was used for scoping to make copy and paste easier so that the lets were local. However since the call value is changing, I could see this in a for or forEach loop, would that still trigger the warning?

test('my test', async function (assert) {
          {
               let spyCall = apiSendRequestStub.getCall(0);
               let { resource, options } = spyCall.args[0];
               assert.strictEqual(resource, 'api/inventory/reconcileAll');
               assert.notOk(options.params.is340B);
         }

          {
               let spyCall = apiSendRequestStub.getCall(1);
               let { resource, options } = spyCall.args[0];
               assert.strictEqual(resource, 'api/inventory/reconcileAll');
               assert.notOk(options.params.is340B);
         }
}

Ember cli page object allows an as function to alias a variable. This also triggers the require-expect

    page.inventoryRows.objectAt(0).as((inventoryRow) => {
      assert.strictEqual(inventoryRow.ndc, '61755-0005-02');
      assert.strictEqual(inventoryRow.productName, 'Eylea 2mg/0.05mL');
      assert.strictEqual(inventoryRow.quantity, '4');
      assert.strictEqual(inventoryRow.unitPrice, '$4,444.00');
      assert.strictEqual(inventoryRow.extendedPrice, '$17,776.00');
    });

This one could be rewritten as below, but obsoletes the as function from page object. Again the index of objectAt could be in a for or forEach which Im not sure if the linter takes into account.

    let inventoryRows = page.inventoryRows.objectAt(0);
    assert.strictEqual(inventoryRows.ndc, '61755-0005-02');
    assert.strictEqual(inventoryRows.productName, 'Eylea 2mg/0.05mL');
    assert.strictEqual(x.quantity, '4');
    assert.strictEqual(inventoryRow.unitPrice, '$4,444.00');
    assert.strictEqual(inventoryRow.extendedPrice, '$17,776.00');

[platinumazure]

Hi @cah-brian-gantzler, thanks for the report.

Can you please share what configuration you are using for this rule?

[cah-brian-gantzler]

I think the linter was installed by ember-cli-update. I changed no files so should be whatever the default is. What file should I look at to give you the configuration?

[bmish]

Ember just uses the recommended config and thus the default config for each rule:

https://github.com/ember-cli/ember-cli/blob/018a541a0a283d769bbe636472d1f49f350aa25c/blueprints/app/files/.eslintrc.js#L66

Note that we are planning to change the default config for this require-expect rule to never-except-zero as mentioned in #222.

[platinumazure]

I agree that except-simple should handle blocks that aren't conditionally executed. The intention is to require expect when someone puts an assertion inside an if statement or other conditional logic.

As for function callbacks, we have no way to infer statically that the callback is always executed. So we would probably need to consider enhancing the rule to support user-supplied function names that the rule can "trust". I don't remember if we have an open issue for that or not (I'm on mobile so I can't check easily while writing this).

I definitely consider the block case as a bug and would happily review any contributions to fix. Thanks!

[cah-brian-gantzler]

Im assuming for and forEach would be the same thing. Might want to put them in the list of "trust" functions

[platinumazure]

@cah-brian-gantzler I'm not sure I agree with that because the number of iterations might not be statically determinable. "except-simple" means we want to see assert.expect in all but the most obvious cases.

You can always override your config to use a different, more lenient setting (such as never-except-zero).

[platinumazure]

@cah-brian-gantzler Do you still believe a change is needed to this rule? Or does the never-except-zero option work for your use case?

[cah-brian-gantzler]

Have thought about this a lot and converted several tests to what works, doesnt work. Yes I believe that never-except-zero will work for me. In many cases where we were using expect could really be written a different way where expect was not required and actually made for better tests.

Thanks for asking.