Page MenuHomePhabricator
Differential D14423

[mock-chronik-client] Add module to monorepo
ClosedPublic
Actions

Authored by emack on Aug 29 2023, 09:22.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Commits
rABC9fbe3a8d720f: [mock-chronik-client] Add module to monorepo
Summary

As per feedback from D14418, many eCash apps are using an identical version of this mock chronik client, often tailored to specific needs.
This module is a fully fleshed out mock across the chronik-client APIs.

Separate diffs will be made to update existing apps in the monorepo to use this mock implementation.

Test Plan

npm test
plonk README into https://jbt.github.io/markdown-editor/

Diff Detail

Repository
rABC Bitcoin ABC
Branch
mockChronikClient
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 24927
Build 49442: Build Diffapp-dev-examples
Build 49441: arc lint + arc unit

Event Timeline

emack created this revision.Aug 29 2023, 09:22
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 29 2023, 09:22
emack requested review of this revision.Aug 29 2023, 09:22
Harbormaster completed remote builds in B24880: Diff 41980.Aug 29 2023, 09:22
bytesofman requested changes to this revision.Aug 29 2023, 15:08
  1. We don't need this to be an npm module. While this would be useful, in that third parties could use it, the goal is to get this functionality into the existing chronik-client npm module. So we would just have to deprecate it. Instead, it should be imported by relative path in the monorepo.
  2. Rename chronikMock.js to index.js (same with test file)
  3. Implement this in at least one app so we know it is working as intended, say coin-select which was the original diff
apps/mock-chronik-client/README.md
7 ↗(On Diff #41980)

This shouldn't be an npm module

This revision now requires changes to proceed.Aug 29 2023, 15:08
emack updated this revision to Diff 41997.Aug 30 2023, 01:29
  • Removed npm installation instructions in README. Kept package.json as it's needed for the unit test suite and potential inclusion in CI.
  • Renamed chronikMock.js to index.js, and chronikMock.test.js to index.test.js
  • Updated apps/examples/sendXec.test.js to use this chronik mock to show script(), utxos() and broadcastTx() endpoints working fine. Couldn't use ecash-coinselect as an example because its current state in master does not make API calls.
Harbormaster completed remote builds in B24893: Diff 41997.Aug 30 2023, 01:30
bytesofman requested changes to this revision.Aug 30 2023, 12:47
bytesofman added inline comments.
apps/mock-chronik-client/package.json
18 ↗(On Diff #41997)

not sure why this is here, isn't this the same as npm test?

apps/mock-chronik-client/test/index.test.js
89 ↗(On Diff #41997)

add test for an error response here

101 ↗(On Diff #41997)

add test for error response

This revision now requires changes to proceed.Aug 30 2023, 12:47
emack updated this revision to Diff 42017.Aug 30 2023, 15:14
emack marked 3 inline comments as done.
  • Added tests to mock API errors for utxos(), history() and script().
  • Updated package.json
Harbormaster completed remote builds in B24907: Diff 42017.Aug 30 2023, 15:15
bytesofman requested changes to this revision.Aug 30 2023, 19:15
bytesofman added inline comments.
apps/mock-chronik-client/index.js
52–54 ↗(On Diff #42017)

not sure we need this?

as far as I know, we only need use cases where the user is getitng an error from a chronik call, not where the user is passing an Error instead of p2pkh or p2sh

apps/mock-chronik-client/test/index.test.js
228–237 ↗(On Diff #42017)

remove

This revision now requires changes to proceed.Aug 30 2023, 19:15
emack updated this revision to Diff 42030.Aug 30 2023, 23:52
emack marked 2 inline comments as done.

Removed script error mocks

Harbormaster completed remote builds in B24918: Diff 42030.Aug 30 2023, 23:53
bytesofman requested changes to this revision.Aug 31 2023, 19:17

catching some bugs in the existing mocked chronik here

apps/mock-chronik-client/index.js
30 ↗(On Diff #42030)

all API call functions must be async

93 ↗(On Diff #42030)

async

98 ↗(On Diff #42030)

this and other setting functions can stay sync

130 ↗(On Diff #42030)

async

This revision now requires changes to proceed.Aug 31 2023, 19:17
emack updated this revision to Diff 42041.Sep 1 2023, 00:50
emack marked 4 inline comments as done.

Updated mock API calls to async

apps/mock-chronik-client/index.js
93 ↗(On Diff #42030)

Close is not an async call as per the chornik client codebase. The only ws async call is waitForOpen() which I've now made async.

apps/mock-chronik-client/package.json
18 ↗(On Diff #41997)

Removed

Harbormaster completed remote builds in B24927: Diff 42041.Sep 1 2023, 00:51
bytesofman accepted this revision.Sep 5 2023, 12:44
This revision is now accepted and ready to land.Sep 5 2023, 12:44
Closed by commit rABC9fbe3a8d720f: [mock-chronik-client] Add module to monorepo (authored by emack). · Explain WhySep 5 2023, 13:06
This revision was automatically updated to reflect the committed changes.
emack added a commit: rABC9fbe3a8d720f: [mock-chronik-client] Add module to monorepo.
emack mentioned this in D14417: [ecash-coinselect] Update getInputUtxos to support custom outputs and OP_RETURN bytecounts.Sep 5 2023, 14:30

Revision Contents
Changeset List

PathSize
apps/
examples/
test/
2 lines
mock-chronik-client/
8 lines
91 lines
164 lines
mocks/
4783 lines
32 lines
test/
252 lines

Diff 42041

View Options

apps/examples/test/sendXec.test.js

Loading...
View Options

apps/mock-chronik-client/.gitignore

Loading...
View Options

apps/mock-chronik-client/README.md

Loading...
View Options

apps/mock-chronik-client/index.js

Loading...
View Options

apps/mock-chronik-client/mocks/mockChronikResponses.js

Loading...
View Options

apps/mock-chronik-client/package-lock.json

Loading...
View Options

apps/mock-chronik-client/package.json

Loading...
View Options

apps/mock-chronik-client/test/index.test.js

Loading...