Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
test: set ESM_DISABLE_CACHE to maybe fix CI-vs--r-esm
We were seeing random test failures in the zoe unit tests (specifically
test-offerSafety.js) that looked like:
```
Uncaught exception in test/unitTests/test-offerSafety.js
/home/runner/work/agoric-sdk/agoric-sdk/packages/weak-store/src/weakStore.js:5
import { assert, details, q } from '@agoric/assert';
^^^^^^
SyntaxError: Cannot use import statement outside a module
✖ test/unitTests/test-offerSafety.js exited with a non-zero exit code: 1
```
or:
```
Uncaught exception in test/unitTests/test-offerSafety.js
/home/runner/work/agoric-sdk/agoric-sdk/packages/zoe/test/unitTests/setupBasicMints.js:1
import { makeIssuerKit } from '@agoric/ertp';
SyntaxError: Cannot use import statement outside a module
✖ test/unitTests/test-offerSafety.js exited with a non-zero exit code: 1
```
under various versions of Node.js. The error suggests that `-r esm` was not
active (the error site tried to import an ESM-style module, and failed
because the import site was CJS not ESM), however error site itself was in a
module, meaning `-r esm` was active up until that moment. This makes no
sense.
The AVA runner has had some amount of built-in support for ESM, in which it
uses Babel to rewrite test files (but perhaps not the code being tested). If
that were in effect, it might explain how `test-offerSafety.js` could be
loaded, but `weakStore.js` could not. It is less likely to explain how
`setupBasicMints.js` could be loaded but `makeIssuerKit` could not, unless
AVA somehow believes that `setupBasicMints.js` is a different category of
file than the ones pulled from other packages.
In any case, I believe we're bypassing that built-in/Babel support, by using
their recommended `-r esm` integration recipe (package.json has
`ava.require=['esm']`). However the AVA "ESM Plan"
(avajs/ava#2293) is worth reading, if only
for our future move-to-native-ESM plans (#527).
So my hunch here is that the `-r esm` module's cache is not safe against
concurrent access, and AVA's parallel test invocation means there are
multiple processes reading and writing to that cache in parallel. Zoe has
more source files than most other packages, which might increase the
opportunity for a cache-corruption bug to show up. This sort of bug might not
show up locally because the files are already in the cache, whereas CI may
not already have them populated.
This patch adds `ESM_DISABLE_CACHE=true` to the environment variables used in
all tests, in an attempt to avoid this hypothetical bug. Stale entries in
this cache has caused us problems before, so most of us have the same setting
in our local shells.
Another potential workaround would be to add `--serial` to the `ava`
invocation, however the Zoe test suite is large enough that we really to want
the parallelism, just to make the tests finish faster.
This patch also increases the Zoe test timeout to 10m, just in case. I
observed a few tests taking 1m30s or 1m40s to complete, and the previous
timeout was 2m, which was too close to the edge.- Loading branch information
