The shared mocks used previously in the incoming transaction controller
tests have been replaced with functions that can generate a new mock
for each test.
We should avoid ever sharing mocks between tests. It's quite easy for
a mock to get accidentally mutated or not correctly "reset" for the
next test, leading to test inter-dependencies and misleading results.
In particular, it is unsafe to share a `sinon` fake (e.g. a spy or
stub) because they can't be fully reset between tests. Or at least it's
difficult to reset them property, and it can't be done while also
following their recommendations for preventing memory leaks.
The spy API and all related state can be reset with `resetHistory`,
which can be called between each test. However `sinon` also recommends
calling `restore` after each test, and this will cause `sinon` to drop
its internal reference to the fake object, meaning any subsequent call
to `resetHistory` would fail. This is intentional, as it's required to
prevent memory from building up during the test run, but it also means
that sharing `sinon` fakes is particularly difficult to do safely.
Instead we should never share mocks in the first place, which has other
benefits anyway.
This was discovered while writing tests for #9583. I mistakenly
believed that `sinon.restore()` would reset the spy state, and this was
responsible for many hours of debugging test failures.
Mark Stacey
d99d8591f0