Page MenuHomePhabricator
Differential D14230

[ecash-herald] Use saved mocks if available
ClosedPublic
Actions

Authored by bytesofman on Jul 7 2023, 17:37.

Details

Reviewers
PiRK
Group Reviewers
Restricted Project
Commits
rABC1c058cec5250: [ecash-herald] Use saved mocks if available
Summary

Testing of ecash-herald is complicated by API calls used in the npm run generateMocks script. We aren't really interested in these changing API call results (e.g. the utxo set of tested addresses, crypto prices). Some API calls will never change so we shouldn't be making them all the time (blockDetails).

This diff checks blocks.js for saved mocks for a specific blockhash. If they exist, these saved mocks are used instead of live API calls. If they do not exist, live API calls are used.

Test Plan

npm run generateMocks true and blocks.js does not change

Diff Detail

Repository
rABC Bitcoin ABC
Branch
herald-mocks-from-constants
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 24408
Build 48416: Build Diffecash-herald-tests
Build 48415: arc lint + arc unit

Event Timeline

bytesofman created this revision.Jul 7 2023, 17:37
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 7 2023, 17:37
bytesofman requested review of this revision.Jul 7 2023, 17:37
bytesofman planned changes to this revision.Jul 7 2023, 17:38
Harbormaster completed remote builds in B24406: Diff 41320.Jul 7 2023, 17:39
bytesofman updated this revision to Diff 41321.Jul 7 2023, 17:46

Test blocks as params

Harbormaster completed remote builds in B24407: Diff 41321.Jul 7 2023, 17:47
bytesofman added a comment.Jul 7 2023, 17:53

Note: I considered adding unit tests to the generateMocks function, but imo it is not worth it...since the goal of modifying the function is to avoid API calls, I don't want to test it by seeing if it does make API calls when expected.

generateMocks is used nowhere in the production of the app and is a developer-only tool. I've added usage documentation in the README.

bytesofman updated this revision to Diff 41322.Jul 7 2023, 17:53

Improve documentation

bytesofman edited the test plan for this revision. (Show Details)Jul 7 2023, 17:53
Harbormaster completed remote builds in B24408: Diff 41322.Jul 7 2023, 17:54
PiRK accepted this revision.Jul 9 2023, 20:31
PiRK added a subscriber: PiRK.
PiRK added inline comments.
apps/ecash-herald/scripts/generateMocks.js
190–210

This block could probably be deduplicated. Something like

let chronikApi = chronik;          
           
// Mock coingecko price response
// onNoMatch: 'throwException' helps to debug if mock is not being used
const mock = new MockAdapter(axios, {
    onNoMatch: 'throwException',
});

if (thisSavedBlock.length > 0) {    # previously line 135
    ...
    chronikApi = mockedChronik;
}    # line 202
handleBlockConnectedPromises.push(
    returnHandleBlockConnectedPromise(
        chronikApi,
        telegramBot,
        mockChannelId,
        blockhash,
        true,
        blockname,
    ),
);
This revision is now accepted and ready to land.Jul 9 2023, 20:31
bytesofman updated this revision to Diff 41353.Jul 10 2023, 16:18

Remove duplicity

Harbormaster completed remote builds in B24430: Diff 41353.Jul 10 2023, 16:19
Closed by commit rABC1c058cec5250: [ecash-herald] Use saved mocks if available (authored by bytesofman). · Explain WhyJul 10 2023, 16:21
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABC1c058cec5250: [ecash-herald] Use saved mocks if available.

Revision Contents
Changeset List

PathSize
apps/
ecash-herald/
18 lines
scripts/
169 lines
test/
mocks/

Diff 41322

View Options

apps/ecash-herald/README.md

Loading...
View Options

apps/ecash-herald/scripts/generateMocks.js

Loading...
View Options

apps/ecash-herald/test/mocks/blocks.js

Loading...