[cashtab] Move the chronik urls config out of Ticker.js
ClosedPublic
Actions

Authored by Fabien on Jun 22 2023, 07:47.

Details

Reviewers

bytesofman

Group Reviewers

Restricted Project

Commits

rABCc89f23c8e027: [cashtab] Move the chronik urls config out of Ticker.js

Summary

The Ticker.js file is a big ball of mud that contains every config items including (and especially) the ones that are unrelated to tickers.

This diffs only move the chronik urls config out of this mess, but more similar extractions needs to happen.

Test Plan

npm test # With this patch it fails on my machine the same way that it fails on master

Diff Detail

Repository

rABC Bitcoin ABC

Branch

chronik_config

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 24130
Build 47870: Build Diff	cashtab-tests
Build 47869: arc lint + arc unit

Event Timeline

Fabien created this revision.Jun 22 2023, 07:47

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 22 2023, 07:47

Fabien requested review of this revision.Jun 22 2023, 07:47

Fabien added a child revision: D14103: [cashtab] Add chronik2.fabien.cash as a backup.Jun 22 2023, 07:49

Harbormaster completed remote builds in B24130: Diff 40924.Jun 22 2023, 07:55

With this patch it fails on my machine the same way that it fails on master

Both master and this patch passed all ok on my end, only thing I can think of is node related, what's your node version? I tested against both v15 and v16 and both were all ok.

Other tests on this patch:

grep -r chronikUrls src/ and ensured no legacy references to chronikUrls in Ticker.js throughout the app
npm start and ensure no chronik errors from useWallet's tryNextChronikUrl logic
send a XEC tx and ensure successful broadcast via the chronik-client

Both master and this patch passed all ok on my end, only thing I can think of is node related, what's your node version? I tested against both v15 and v16 and both were all ok.

$ node --version
v15.14.0

I see what's going on, all the snapshots are wrong because I'm using a locale that uses , as a decimal separator instead of ..
Is there any way to force npm to run the tests with some defined locale without overriding the system locale ?

emack mentioned this in D14109: [Cashtab] Move the token urls config out of Ticker.js.Jun 23 2023, 14:33

emack mentioned this in D14110: [Cashtab] Move the websocket config out of Ticker.js.Jun 23 2023, 15:08

In D14102#315017, @Fabien wrote:

I see what's going on, all the snapshots are wrong because I'm using a locale that uses , as a decimal separator instead of ..
Is there any way to force npm to run the tests with some defined locale without overriding the system locale ?

I remember us running into this issue a long time ago. It looks like this fix was not kept through the webpack / react upgrade.

See if D14111 tests pass on your machine

Also note that we are on node 16 now, though this is probably not the cause of this issue.

In D14102#315179, @bytesofman wrote:

In D14102#315017, @Fabien wrote:

I see what's going on, all the snapshots are wrong because I'm using a locale that uses , as a decimal separator instead of ..
Is there any way to force npm to run the tests with some defined locale without overriding the system locale ?

I remember us running into this issue a long time ago. It looks like this fix was not kept through the webpack / react upgrade.

See if D14111 tests pass on your machine

Also note that we are on node 16 now, though this is probably not the cause of this issue.