chore: retry failed Circle CI tests

[nolanlawson]

Details

Retries Circle CI karma/integration tests until they pass.

Does this pull request introduce a breaking change?

• ✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

• ✅ No, it does not introduce an observable change.

GUS work item

W-10946477

[nolanlawson]

I checked that the correct env vars are being passed in to the CLI commands, and the total computed coverage is the same before and after, so I think this is good to go.

Basically it does 3 retries before failing. It's a bit of boilerplate in the Circle CI config, but other than that it's pretty straightforward.

[ravijayaramappa]

I think the goal of this routine is to retry the entire command so it works for both karma and integration suites. But I was wondering if we can collect only the failed tests and rerun them instead of rerunning the entire command.

One problem with that could be that the coverage numbers of the most recent partial run might overwrite the prior full run.

[ravijayaramappa]

We could use like a Junit style reporter https://www.npmjs.com/package/karma-junit-reporter and gather the specs that have a failure and rerun those.

[nolanlawson]

Some test runners have a built-in retry mechanism, e.g. Cypress retries or TestCafe quarantine mode. Unfortunately I couldn't find anything similar for Karma – if it existed, I would definitely prefer to use it. It does have a retryLimit, but this appears to only apply to browser crashes.

[pmdartus]

Nitpick: It feels strange that the generic retry_until_success command accepts coverage, compat, disable_synthetic, and force_native_shadow_mode as parameters since those parameters are only applicable to the Karma tests.

IMO it would be preferable to keep them inside the Kama command. It appears the CircleCi config syntax doesn't accept passing arbitrary objects. Many passing them as via a generic environment parameter would do the trick.

[nolanlawson]

I considered using environment, but it's tricky because right now we are relying on the presence or absence of environment variables. E.g. I'm not sure if passing COVERAGE=0 will actually disable coverage – I suspect it would be converted to a string "0" in JavaScript, which is truthy, so coverage would be enabled. We would have to audit each of our environment variables to make sure they handle "0", "false", etc.

I agree the name retry_until_success is not great. Maybe retry_karma_until_success? Or just retry_karma?

[pmdartus]

Maybe retry_karma_until_success? Or just retry_karma?

retry_karma sounds good.

We would have to audit each of our environment variables to make sure they handle "0", "false", etc.

You can set those environment variables to a nullish value like an empty string, which would coerce to a false boolean.

COVERAGE="" COMPAT="" yarn test

While looking into this, I learned that bash allows you to define environment variables without any value. The following code would also work.

COVERAGE= COMPAT= yarn test

Again it's a nitpick.

[nolanlawson]

Renamed to retry_karma. I think I'd like to keep the existing parameter system, because it's a little unclear to me from reading the Circle CI docs how to pass an environment variable to a command. If there's a good way to refactor this, let's do it in a follow-up PR. 🙂