Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC3b3010946eda: [herald] Unit tests for parseWebsocketMessage

Summary

T2972

Depends on D13616

Add unit tests for parseWebsocketMessage

These unit tests required a mock Telegram bot and improvements to chronikMock.js

For now, I think it's okay to maintain chronikMock.js as distinct files in both alias-server and ecash-herald. The incremental work in both apps is a good way to learn what features this mock needs. It will eventually be extended to Cashtab and live as a centralized dependency.

Test Plan

npm test

Diff Detail

Repository

rABC Bitcoin ABC

Branch

herald-refactor-stack

Lint

Lint Errors

Severity	Location	Code	Message
Error	apps/ecash-herald/test/mocks/telegramBotMock.js:11	ESLINT	no-unused-vars

Unit

No Test Coverage

Build Status

Buildable 23080
Build 45780: Build Diff	ecash-herald-tests
Build 45779: arc lint + arc unit

Event Timeline

bytesofman created this revision.Apr 7 2023, 17:33

Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 7 2023, 17:33

bytesofman requested review of this revision.Apr 7 2023, 17:33

Harbormaster completed remote builds in B23080: Diff 39405.Apr 7 2023, 17:33

Clean up comments in telegramBotMock.js

Harbormaster completed remote builds in B23081: Diff 39406.Apr 7 2023, 17:36

Check if telegramBot called or did not call expected routine

Harbormaster completed remote builds in B23082: Diff 39407.Apr 7 2023, 17:39

rebase

Harbormaster completed remote builds in B23084: Diff 39409.Apr 7 2023, 17:42

bytesofman added a child revision: D13618: [tg bot] Unit test for main().Apr 7 2023, 18:12

Fabien requested changes to this revision.Apr 9 2023, 06:42

Fabien added a subscriber: Fabien.

Fabien added inline comments.

apps/ecash-herald/src/websocket.js
79 ↗	(On Diff #39409)	just `return await telegramBot.sendMessage(...)`
88 ↗	(On Diff #39409)	that's unrelated
89 ↗	(On Diff #39409)	you're missing a return false, either here or at the end of the function (then you don't need it in the default: case)
apps/ecash-herald/test/mocks/telegramBotMock.js
6 ↗	(On Diff #39409)	It's not a good idea to rely on a sample file, people typically delete/rename these. Can't you just hardcode some default dummy values in the mock?
apps/ecash-herald/test/websocketTests.js
158 ↗	(On Diff #39409)	You are missing a test case: returns false if send_message failed