Add support and tests for new plugins endpoint in chronik-client
Details
Details
- Reviewers
tobias_ruck - Group Reviewers
Restricted Project - Commits
- rABC179560629131: [chronik-client] Support plugins
CI tests
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- cc-plugins-support
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Event Timeline
Comment Actions
Tail of the build log:
Running Unit Tests for Test Framework Modules [1msetup_scripts/chronik-client_mempool_conflicts.py[0m started [1msetup_scripts/chronik-client_mempool_conflicts.py[0m passed, Duration: 1 s [1m TEST | STATUS | DURATION [0m[0;32msetup_scripts/chronik-client_mempool_conflicts.py | ✓ Passed | 1 s [0m[1m ALL | ✓ Passed | 1 s (accumulated) [0mRuntime: 1 s testRunner complete in mempool_conflicts.ts Starting test_runner for chronik-client_plugin_groups Test runner for chronik-client_plugin_groups started 2024-08-07T22:33:04.123000Z TestFramework (WARNING): Test Skipped: Chronik indexer plugins have not been compiled. 2024-08-07T22:33:04.173000Z TestFramework (INFO): Stopping nodes 2024-08-07T22:33:04.173000Z TestFramework (INFO): Cleaning up /work/abc-ci-builds/chronik-client-integration-tests/test/tmp/test_runner_₿₵_🏃_20240807_223303/setup_scripts/chronik-client_plugin_groups_0 on exit 2024-08-07T22:33:04.173000Z TestFramework (INFO): Test skipped Running Unit Tests for Test Framework Modules [1msetup_scripts/chronik-client_plugin_groups.py[0m started [1msetup_scripts/chronik-client_plugin_groups.py[0m skipped [1m TEST | STATUS | DURATION [0m[1;30msetup_scripts/chronik-client_plugin_groups.py | ○ Skipped | 0 s [0m[1m ALL | ✓ Passed | 0 s (accumulated) [0mRuntime: 0 s plugin_groups.ts tests complete, shutting down child process Test runner error for chronik-client_plugin_groups, aborting: Error [ERR_IPC_CHANNEL_CLOSED]: Channel closed -----------------------|---------|----------|---------|---------|----------------------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s -----------------------|---------|----------|---------|---------|----------------------------------- All files | 19.01 | 5.03 | 15.48 | 18.94 | chronik-client | 100 | 100 | 100 | 100 | index.ts | 100 | 100 | 100 | 100 | chronik-client/proto | 14.58 | 3.8 | 9.14 | 14.68 | chronik.ts | 6.1 | 0.83 | 2.54 | 6.09 | ...,3978-3985,3990-4027,4031-4036 chronikNode.ts | 19.81 | 5.97 | 13.52 | 19.99 | ...,5990-6029,6037-6110,6145-6154 chronik-client/src | 44.54 | 22.18 | 41.17 | 43.75 | ChronikClient.ts | 4.24 | 0 | 0 | 4.29 | 33-163,178-222,290-692 ChronikClientNode.ts | 47.32 | 33.08 | 52.8 | 47.32 | ...,1182,1192,1213-1221,1229-1294 failoverProxy.ts | 75.22 | 51.61 | 62.06 | 74.52 | ...67,275-285,294,301,305,310,314 hex.ts | 89.47 | 50 | 75 | 87.87 | 58,66-68 validation.ts | 66.66 | 18.18 | 66.66 | 62.96 | 17,21,33,38-49,62-63 -----------------------|---------|----------|---------|---------|----------------------------------- ##teamcity[blockOpened name='Code Coverage Summary'] ##teamcity[buildStatisticValue key='CodeCoverageAbsBCovered' value='880'] ##teamcity[buildStatisticValue key='CodeCoverageAbsBTotal' value='4629'] ##teamcity[buildStatisticValue key='CodeCoverageAbsRCovered' value='214'] ##teamcity[buildStatisticValue key='CodeCoverageAbsRTotal' value='4249'] ##teamcity[buildStatisticValue key='CodeCoverageAbsMCovered' value='133'] ##teamcity[buildStatisticValue key='CodeCoverageAbsMTotal' value='859'] ##teamcity[buildStatisticValue key='CodeCoverageAbsLCovered' value='868'] ##teamcity[buildStatisticValue key='CodeCoverageAbsLTotal' value='4582'] ##teamcity[blockClosed name='Code Coverage Summary'] mv: cannot stat 'test_results/chronik-client-integration-tests-junit.xml': No such file or directory Build chronik-client-integration-tests failed with exit code 1
| modules/chronik-client/proto/chronikNode.ts | ||
|---|---|---|
| 308 ↗ | (On Diff #49092) | This file is generated will need to add support for the new blockheader endpoints in a separate diff. at the moment I'm not aware of a chronik-client use case for this but doesn't hurt to have it available. |
| modules/chronik-client/src/ChronikClientNode.ts | ||
| 1011 ↗ | (On Diff #49092) | In this way, we get groups as an array of hex strings, ['aaa', 'aab', 'aba'], instead of the object array format in proto, [{group: 'aaa'}, {group: 'aab'}, {group: 'aba'}] Is there some reason the object array format should be preserved here in chronik-client? Mb more info will be added later? |
| modules/chronik-client/src/ChronikClientNode.ts | ||
|---|---|---|
| 1011 ↗ | (On Diff #49092) | I made it its own independent message in case we wanna add more fields in the future, e.g. the number of UTXOs or txs of the group. I think it would be better to mimick the proto so we have the extensibility if needed. |
Comment Actions
preserve group object for extensibility, remove unused constants from plugin_groups.ts test file, rebase
| modules/chronik-client/test/integration/plugin_groups.ts | ||
|---|---|---|
| 113 ↗ | (On Diff #49100) | It should return 404: Plugin "<plugin_name>" not loaded — maybe the client needs to have some special handling? |
| modules/chronik-client/test/integration/plugin_groups.ts | ||
|---|---|---|
| 113 ↗ | (On Diff #49100) | typo in test implementation, corrected |
Comment Actions
some ideas for better code style
| modules/chronik-client/src/ChronikClientNode.ts | ||
|---|---|---|
| 983 | wouldn't it make sense to use Object.entries here? | |
| 984 | I think it would be better style to use a continue here to save the indent | |
| 987 | if you use the .map as below, you don't need an extra variable | |
| 988 | wouldn't this be cleaner? dito for data | |
| 1015 | less convinced of this one, but could be cleaner; up to you | |
| modules/chronik-client/src/ChronikClientNode.ts | ||
|---|---|---|
| 983 | not sure what we need entries for vs plugins. we don't need the values; if there are no keys then there are no plugins. is there some advantage i'm overlooking? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries | |
| 988 | yes this is much better | |
| modules/chronik-client/src/ChronikClientNode.ts | ||
|---|---|---|
| 983 | well you can do this right away: for (const [pluginName, plugin] of Object.entries(plugins) { and then you don't have to do the plugins[plugin] later because you already have the plugin | |
Comment Actions
if you don't like the Object.entries change, fine to ignore
| modules/chronik-client/test/integration/plugins.ts | ||
|---|---|---|
| 377–378 ↗ | (On Diff #49114) | every other place is ordered groups, data |
| modules/chronik-client/src/ChronikClientNode.ts | ||
|---|---|---|
| 983 | ohhh I was misreading this, for not-paying-attention reasons I thought the comment was on if (Object.keys(utxo.plugins).length > 0) {
// We only return a plugins key if we have plugins
utxoInNode.plugins = convertToPluginEntries(utxo.plugins);
}yeah, Object.entries is better here, replaced | |