Some of the unit tests for the incoming transaction controller included
a 1 second wait. The wait was to ensure that a state update did not
occur, as it happens asynchronously.
The tests work equally well using a `setTimeout` with a zero second
wait, because the asynchronous block update is guaranteed to have been
queued up by the time this timeout function is called. The timeout has
been reduced to `0` to speed up the tests.
Additionally, `undefined` has been added to the list of network names
used to construct the fake API responses. This is to ensure that the
API returns a valid response, so that the test fails when it should.
It was getting rather complicated to keep track of which Segment keys
were set where, and under which name.
The build script now injects a key even in test environments, but it is
unused if `IN_TEST` is truthy. This should be functionally equivalent
to the old logic. I find this simpler mainly for two reasons: there is
one less intermediate variable to keep track of now, and the `IN_TEST`
check is now directly in the module where we're constructing the
`segment` instance, rather than being referenced at a distance in a
comment.
The old setup made it difficult to turn on metrics for specific e2e
tests as well, which will be done in a subsequent PR.
There were a few lint errors introduced in #9768 due to the recently
merged prettier PR. They have now been fixed.
Additionally, one line using the property `eth_accounts` was
intermittently failing for me locally. I've been seeing this lint
failure off-and-on for a few days now - I'm not sure why I haven't seen
it on CI. Either way though, it's now ignored.
The incoming transactions controller now uses the `chainId` for the
current network instead of the `networkId`. This ensures that custom
RPC endpoints for the built-in supported networks do correctly receive
incoming transactions.
As part of this change, the incoming transactions controller will also
cease keeping track of the "last block fetched" for networks that are
not supported. This piece of state never really represented the last
block fetched, as _no_ blocks were fetched for any such networks. It
been removed.
Unit tests have been added to the incoming transactions controller to
ensure that block updates are correctly resulting in state updates when
incoming transactions are enabled. All other events that trigger state
updates are tested as well.
The tests were written to be minimally dependent upon implementation
details of the controller itself. `nock` was used to mock the API
response from Etherscan. Each event is triggered asynchronously by
`sinon`, as in production they are likely only triggered
asynchronously.
This was extracted from #9583
This PR includes a new `wait-until-called` module meant to help with
writing asynchronous tests. It allows you to wait until a stub has been
called.
* Standardize appearance of network settings
* Add separate route for network settings form
* Control network form rendering in popup via route
* Hide network form buttons when form is viewOnly
* Handle extremely long network names
If the swaps state is cleared in between the initial quote fetch and
the subsequent poll fetch, a `TypeError` will be thrown due to
`fetchParams` being set to `null`.
This is of no functional consequence, as `fetchParams` _should_ be
`null` in this case, and and no further action should be taken.
The optional chaining operator is now used to ensure the call no longer
throws.
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.
The changelog has been updated with one minor UX improvement, and the
entry regarding failed swap fetches has been updated to point at the PR
that actually fixed the issue.
The v8.1.3 changelog has been updated with all user-facing changes
included with v8.1.3
PR #9612 was left under "Current Develop Branch" because it has been
temporarily reverted for this release. It was added there now so that
we don't forget about it, as the revert might result in that commit
not being populated by the changelog script for the next release.
