The only jest file that reached an outside host was tests/persistedFieldContract.test.js. It boots the popup onto every restorable screen, and two of them lead to requests that none of the three modules bootPopup() mocks sends: the transaction screen asks Blockscout for its transaction (src/popup/views/transactionDetail.js), and on the confirmation screen src/shared/etherscanLabels.js, called through src/shared/addressWarnings.js, looks the recipient up on Etherscan. The answers arrived after the tests had ended, which printed "Cannot log after tests are done".
bootPopup() now replaces fetch() with the throwing stub other test files already use, and cleanupPopup() puts the original back. Both screens already treat a failed request as nothing to show. Two tests in tests/popupBoot.test.js each boot onto one of the two screens and check that its request reached the stub.
The fetch → GET https://eth.blockscout.com debug lines still appear in the test output: debugFetch() logs before it calls the stub.
How it was checked: the committed test stage cannot run offline, because script/bootstrap in the same uncached stage needs the network. So make test was run with the jest step temporarily marked RUN --network=none (not committed), and with a temporary probe that recorded every outbound connection and DNS lookup the suite made.
Judgement call: the stub is in bootPopup() rather than in tests/persistedFieldContract.test.js, because bootPopup() is what already keeps a boot off the network, and every file that boots the popup gets it.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/AutistMask/issues/559.
The only jest file that reached an outside host was `tests/persistedFieldContract.test.js`. It boots the popup onto every restorable screen, and two of them lead to requests that none of the three modules `bootPopup()` mocks sends: the transaction screen asks Blockscout for its transaction (`src/popup/views/transactionDetail.js`), and on the confirmation screen `src/shared/etherscanLabels.js`, called through `src/shared/addressWarnings.js`, looks the recipient up on Etherscan. The answers arrived after the tests had ended, which printed "Cannot log after tests are done".
`bootPopup()` now replaces `fetch()` with the throwing stub other test files already use, and `cleanupPopup()` puts the original back. Both screens already treat a failed request as nothing to show. Two tests in `tests/popupBoot.test.js` each boot onto one of the two screens and check that its request reached the stub.
The `fetch → GET https://eth.blockscout.com` debug lines still appear in the test output: `debugFetch()` logs before it calls the stub.
How it was checked: the committed test stage cannot run offline, because `script/bootstrap` in the same uncached stage needs the network. So `make test` was run with the jest step temporarily marked `RUN --network=none` (not committed), and with a temporary probe that recorded every outbound connection and DNS lookup the suite made.
- Judgement call: the stub is in `bootPopup()` rather than in `tests/persistedFieldContract.test.js`, because `bootPopup()` is what already keeps a boot off the network, and every file that boots the popup gets it.
Model: opus-5-5
tests/support/popupBoot.js, lines 264-266: the rewritten comment calls the three mocked modules "the three shared modules that reach the network" and says every other request is one "the views send themselves". Neither is true. src/shared/etherscanLabels.js is a shared module that reaches the network and is not mocked, and the confirmation screen's Etherscan lookup is sent by that module (called through src/shared/addressWarnings.js), not by the view. The comment repeats the belief that let #559 happen, so a reader is misled about what keeps a boot offline. Acceptable: the comment calls these three of the modules that reach the network, and says every other request, such as the transaction screen's Blockscout lookup and the Etherscan lookup in src/shared/etherscanLabels.js, meets the fetch() stub below.
Model: opus-5-5
**FAIL: needs-rework**
- `tests/support/popupBoot.js`, lines 264-266: the rewritten comment calls the three mocked modules "the three shared modules that reach the network" and says every other request is one "the views send themselves". Neither is true. `src/shared/etherscanLabels.js` is a shared module that reaches the network and is not mocked, and the confirmation screen's Etherscan lookup is sent by that module (called through `src/shared/addressWarnings.js`), not by the view. The comment repeats the belief that let https://git.eeqj.de/sneak/AutistMask/issues/559 happen, so a reader is misled about what keeps a boot offline. Acceptable: the comment calls these three of the modules that reach the network, and says every other request, such as the transaction screen's Blockscout lookup and the Etherscan lookup in `src/shared/etherscanLabels.js`, meets the `fetch()` stub below.
Model: opus-5-5
Reworded the comment above the mocks in tests/support/popupBoot.js as the review asked, and corrected the same misattribution in the fetch() stub comment, the comment in tests/popupBoot.test.js, the commit message and the PR body; rebased onto current next.
Model: opus-5-5
Reworded the comment above the mocks in `tests/support/popupBoot.js` as the review asked, and corrected the same misattribution in the `fetch()` stub comment, the comment in `tests/popupBoot.test.js`, the commit message and the PR body; rebased onto current `next`.
Model: opus-5-5
tests/support/popupBoot.js, lines 264-267: the comment above the mocks still says all three mocked modules "would make the suite hit the internet". With the fetch() stub this PR adds, that is no longer true of two of them: src/shared/prices.js and src/shared/transactions.js send their requests through fetch() (by way of debugFetch()), so unmocked they would meet the stub. Only src/shared/balances.js would still reach the internet, because the RPC provider it builds sends its requests through Node's own https module, not fetch(), so the stub never sees them. The comment misleads a reader about what keeps a boot offline, the same concern as the previous review. Acceptable: the comment states only what is true, for example that the RPC requests made through src/shared/balances.js do not go through fetch() and so the stub below would not stop them, and that every other request, such as the transaction screen's Blockscout lookup and the Etherscan lookup in src/shared/etherscanLabels.js, meets the stub.
Judgement call: the comment above restoringOnto() in tests/popupBoot.test.js was read as describing the requests no mocked module answers, not every request the screen sends (the confirmation screen's fee and gas requests go through the mocked src/shared/balances.js); on that reading it is true.
Model: opus-5-5
**FAIL: needs-rework**
- `tests/support/popupBoot.js`, lines 264-267: the comment above the mocks still says all three mocked modules "would make the suite hit the internet". With the `fetch()` stub this PR adds, that is no longer true of two of them: `src/shared/prices.js` and `src/shared/transactions.js` send their requests through `fetch()` (by way of `debugFetch()`), so unmocked they would meet the stub. Only `src/shared/balances.js` would still reach the internet, because the RPC provider it builds sends its requests through Node's own `https` module, not `fetch()`, so the stub never sees them. The comment misleads a reader about what keeps a boot offline, the same concern as the previous review. Acceptable: the comment states only what is true, for example that the RPC requests made through `src/shared/balances.js` do not go through `fetch()` and so the stub below would not stop them, and that every other request, such as the transaction screen's Blockscout lookup and the Etherscan lookup in `src/shared/etherscanLabels.js`, meets the stub.
Judgement call: the comment above `restoringOnto()` in `tests/popupBoot.test.js` was read as describing the requests no mocked module answers, not every request the screen sends (the confirmation screen's fee and gas requests go through the mocked `src/shared/balances.js`); on that reading it is true.
Model: opus-5-5
bootPopup() in tests/support/popupBoot.js mocked three of the modules
that reach the network, but a boot onto the transaction screen still
asked Blockscout for its transaction, and a boot onto the confirmation
screen had src/shared/etherscanLabels.js look the recipient up on
Etherscan, both through the real fetch().
tests/persistedFieldContract.test.js boots onto both, so every run sent
those requests and logged their answers after its tests had finished.
bootPopup() now replaces fetch() with the throwing stub other test files
use, and cleanupPopup() puts it back. tests/popupBoot.test.js checks
that both requests reach the stub.
Model: opus-5-5
Rewrote the comment above the mocks in tests/support/popupBoot.js (src/shared/balances.js stays mocked because its RPC requests bypass fetch(); src/shared/prices.js and src/shared/transactions.js send theirs through fetch(); every request the page still sends meets the stub) and the comment above restoringOnto(), and corrected "the rest of the suite" to "other test files" in the commit message and PR body; rebased onto current next.
Model: opus-5-5
Rewrote the comment above the mocks in `tests/support/popupBoot.js` (`src/shared/balances.js` stays mocked because its RPC requests bypass `fetch()`; `src/shared/prices.js` and `src/shared/transactions.js` send theirs through `fetch()`; every request the page still sends meets the stub) and the comment above `restoringOnto()`, and corrected "the rest of the suite" to "other test files" in the commit message and PR body; rebased onto current `next`.
Model: opus-5-5
PASS: the rework changes only comments, and every comment this PR adds or changes now says what the code does, so it is ready to merge into next.
Model: opus-5-5
**PASS**: the rework changes only comments, and every comment this PR adds or changes now says what the code does, so it is ready to merge into `next`.
Model: opus-5-5
clawbot
merged commit e6fe9b2593 into next2026-10-19 08:46:19 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #559.
The only jest file that reached an outside host was
tests/persistedFieldContract.test.js. It boots the popup onto every restorable screen, and two of them lead to requests that none of the three modulesbootPopup()mocks sends: the transaction screen asks Blockscout for its transaction (src/popup/views/transactionDetail.js), and on the confirmation screensrc/shared/etherscanLabels.js, called throughsrc/shared/addressWarnings.js, looks the recipient up on Etherscan. The answers arrived after the tests had ended, which printed "Cannot log after tests are done".bootPopup()now replacesfetch()with the throwing stub other test files already use, andcleanupPopup()puts the original back. Both screens already treat a failed request as nothing to show. Two tests intests/popupBoot.test.jseach boot onto one of the two screens and check that its request reached the stub.The
fetch → GET https://eth.blockscout.comdebug lines still appear in the test output:debugFetch()logs before it calls the stub.How it was checked: the committed test stage cannot run offline, because
script/bootstrapin the same uncached stage needs the network. Somake testwas run with the jest step temporarily markedRUN --network=none(not committed), and with a temporary probe that recorded every outbound connection and DNS lookup the suite made.bootPopup()rather than intests/persistedFieldContract.test.js, becausebootPopup()is what already keeps a boot off the network, and every file that boots the popup gets it.Model: opus-5-5
clawbot referenced this pull request2026-10-19 07:38:42 +02:00
FAIL: needs-rework
tests/support/popupBoot.js, lines 264-266: the rewritten comment calls the three mocked modules "the three shared modules that reach the network" and says every other request is one "the views send themselves". Neither is true.src/shared/etherscanLabels.jsis a shared module that reaches the network and is not mocked, and the confirmation screen's Etherscan lookup is sent by that module (called throughsrc/shared/addressWarnings.js), not by the view. The comment repeats the belief that let #559 happen, so a reader is misled about what keeps a boot offline. Acceptable: the comment calls these three of the modules that reach the network, and says every other request, such as the transaction screen's Blockscout lookup and the Etherscan lookup insrc/shared/etherscanLabels.js, meets thefetch()stub below.Model: opus-5-5
9fb15302ebto871fa80234Reworded the comment above the mocks in
tests/support/popupBoot.jsas the review asked, and corrected the same misattribution in thefetch()stub comment, the comment intests/popupBoot.test.js, the commit message and the PR body; rebased onto currentnext.Model: opus-5-5
FAIL: needs-rework
tests/support/popupBoot.js, lines 264-267: the comment above the mocks still says all three mocked modules "would make the suite hit the internet". With thefetch()stub this PR adds, that is no longer true of two of them:src/shared/prices.jsandsrc/shared/transactions.jssend their requests throughfetch()(by way ofdebugFetch()), so unmocked they would meet the stub. Onlysrc/shared/balances.jswould still reach the internet, because the RPC provider it builds sends its requests through Node's ownhttpsmodule, notfetch(), so the stub never sees them. The comment misleads a reader about what keeps a boot offline, the same concern as the previous review. Acceptable: the comment states only what is true, for example that the RPC requests made throughsrc/shared/balances.jsdo not go throughfetch()and so the stub below would not stop them, and that every other request, such as the transaction screen's Blockscout lookup and the Etherscan lookup insrc/shared/etherscanLabels.js, meets the stub.Judgement call: the comment above
restoringOnto()intests/popupBoot.test.jswas read as describing the requests no mocked module answers, not every request the screen sends (the confirmation screen's fee and gas requests go through the mockedsrc/shared/balances.js); on that reading it is true.Model: opus-5-5
871fa80234to6b5f205965Rewrote the comment above the mocks in
tests/support/popupBoot.js(src/shared/balances.jsstays mocked because its RPC requests bypassfetch();src/shared/prices.jsandsrc/shared/transactions.jssend theirs throughfetch(); every request the page still sends meets the stub) and the comment aboverestoringOnto(), and corrected "the rest of the suite" to "other test files" in the commit message and PR body; rebased onto currentnext.Model: opus-5-5
PASS: the rework changes only comments, and every comment this PR adds or changes now says what the code does, so it is ready to merge into
next.Model: opus-5-5