Page MenuHomePhabricator
Differential D14330

[Cashtab][Alias] Add chronik mock
ClosedPublic
Actions

Authored by emack on Jul 31 2023, 23:07.

Details

Reviewers
bytesofman
Fabien
Group Reviewers
Restricted Project
Commits
rABC144e5d653d92: [Cashtab][Alias] Add chronik mock
Summary

T3216

This diff separates out the chronik mock from D14325 and updates the registerNewAlias unit tests to be ready for websocket behaviors in D14325.

Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Branch
chronikMock
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 24639
Build 48875: Build Diffcashtab-tests
Build 48874: arc lint + arc unit

Event Timeline

emack created this revision.Jul 31 2023, 23:07
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 31 2023, 23:07
emack requested review of this revision.Jul 31 2023, 23:07
emack mentioned this in D14325: [Cashtab][Alias] Add websocket listener for 1 conf on alias registration txs.Jul 31 2023, 23:13
bytesofman requested changes to this revision.Jul 31 2023, 23:20
bytesofman added inline comments.
cashtab/src/utils/__mocks__/chronikMock.js
8 ↗(On Diff #41655)

limit the logic added here to only what you need for the websocket tests

we have a lot of chronik unit tests in cashtab that should use the mock. Understandably beyond the scope of this diff. difficult to review the mock when it's just dumped in and not implemented.

Can add to this as its implemented in other existing unit tests.

This revision now requires changes to proceed.Jul 31 2023, 23:20
emack updated this revision to Diff 41656.Jul 31 2023, 23:24
emack marked an inline comment as done.

Limiting chronik mock to broadcastTx and websocket logic

Harbormaster completed remote builds in B24638: Diff 41655.Jul 31 2023, 23:26
Harbormaster completed remote builds in B24639: Diff 41656.Jul 31 2023, 23:29
bytesofman requested changes to this revision.Jul 31 2023, 23:37
bytesofman added inline comments.
cashtab/src/utils/__tests__/transactions.test.js
116

why mock this with jest? you have included this method in MockChronikClient, where it can be set by the setMock method

If jest is a better way to do it in Cashtab, don't include these methods in the mock

This revision now requires changes to proceed.Jul 31 2023, 23:37
emack updated this revision to Diff 41657.Jul 31 2023, 23:55
emack marked an inline comment as done.

Removed broadcastTx methods from mock chronik as the jest approach is more realistic

Harbormaster completed remote builds in B24640: Diff 41657.Jul 31 2023, 23:58
bytesofman accepted this revision.Aug 1 2023, 16:35
This revision is now accepted and ready to land.Aug 1 2023, 16:35
Closed by commit rABC144e5d653d92: [Cashtab][Alias] Add chronik mock (authored by emack). · Explain WhyAug 1 2023, 23:14
This revision was automatically updated to reflect the committed changes.
emack added a commit: rABC144e5d653d92: [Cashtab][Alias] Add chronik mock.

Revision Contents
Changeset List

PathSize
cashtab/
src/
utils/
__mocks__/
72 lines
__tests__/
31 lines

Diff 41656

View Options

cashtab/src/utils/__mocks__/chronikMock.js

Loading...
View Options

cashtab/src/utils/__tests__/transactions.test.js

Loading...