Test reorg: Clients, Mirage, General selectors

[hashishaw]

• Rename SELECTORS export to something specific
• Consolidate helpers such as overrideMirageResponse and capabilities stubs
• Remove noop-all-api-requests and rewrite those tests to use mirage

[hashishaw]

This is a longer but more explicit way of checking that the url and data are doing what we expect on login

[github-actions]

CI Results:
All Go tests succeeded! ✅

[github-actions]

Build Results:
All builds succeeded! ✅

[andaley]

there are so many great changes in here! i think pulling the mirage helpers into tests/helpers/stubs really makes a difference. thank you so much for tackling this huge cleanup. 😀

the only changes i think are actually necessary for merging are the comments i prefixed with "🔄 " (mostly easy typo fixes, using assert.true, etc).

aside from that, i had some optional questions about whether we can swap out assert.ok for something more strict. i noticed that we're often using assert.ok inside a stubbed function to verify it was called, which is pretty harmless, but some alternatives to that might be:

// using sinon

const methodStub = sinon.stub();

this.server[method](url, (schema, req) => {
  methodStub();
  assert.strictEqual(req.method, testCase.method, `uses the correct http verb: ${testCase.method}`);
  assert.deepEqual(req.queryParams, queryParams, 'calls with correct query params');
  return {};
});

assert.true(methodStub.calledOnce, `${testCase.adapterMethod} calls the correct url with: ${testCase.url}`);

or

// using a variable flag

    const methodCalled = false;

    this.server[method](url, (schema, req) => {
      functionCalled = true; // set the flag to true when the function is called
      assert.strictEqual(req.method, testCase.method, `uses the correct http verb: ${testCase.method}`);
      assert.deepEqual(req.queryParams, queryParams, 'calls with correct query params');
      return {};
    });

    assert.true(methodCalled, `${testCase.adapterMethod} calls the correct url with: ${testCase.url}`); // Assert that the function was called
  });
});

or if we want to make minimal changes

this.server[method](url, (schema, req) => {
  assert.true(true, 'calls the correct url'); // this will never fail, but that's okay since we're just testing that this line was executed
  assert.strictEqual(req.method, testCase.method, `uses the correct http verb: ${testCase.method}`);
  assert.deepEqual(req.queryParams, queryParams, 'calls with correct query params');
  return {};
});

^ i'm just sharing these ideas for brainstorming purposes right now, so take it or leave it. but i think we should consider setting up linting rules at some point to disallow or warn when using assert.ok. what do you think? we can talk about this at the next team pattern discussion too. 🙂

[andaley]

this is a great start to testing docs! thanks so much for writing these. ✨

[andaley]

is this line a todo item? i'm not sure what it's doing 🤔

[andaley]

nice improvement adding the loop here!

[andaley]

same question here -- wondering if we can find a way to use assert.strictEqual or assert.true 🤔

[andaley]

this one too