Page MenuHomePhabricator

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

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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emack requested review of this revision.Aug 29 2023, 09:22
  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
  • 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.
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 marked 3 inline comments as done.
  • Added tests to mock API errors for utxos(), history() and script().
  • Updated package.json
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 marked 2 inline comments as done.

Removed script error mocks

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 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

This revision is now accepted and ready to land.Sep 5 2023, 12:44
This revision was automatically updated to reflect the committed changes.