Page MenuHomePhabricator

[mock-chronik-client] Install mock-chronik-client dependencies before running tests that require them
ClosedPublic

Authored by bytesofman on Mar 3 2024, 21:59.

Details

Summary

Install mock-chronik-client deps before running js-mocha test suite in repo that depends on them

Test Plan

Test on js-mocha suite that does not require mock-chronik-client: ./contrib/teamcity/build-configurations.py ecashaddrjs-tests
Test on js-mocha suite that does require mock-chronik-client: ./contrib/teamcity/build-configurations.py ecash-herald-tests

Diff Detail

Repository
rABC Bitcoin ABC
Branch
revert-mcc-ci
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 27628
Build 54819: Build Diff
Build 54818: arc lint + arc unit

Event Timeline

bytesofman published this revision for review.Mar 3 2024, 22:00
Fabien requested changes to this revision.Mar 4 2024, 09:02
Fabien added a subscriber: Fabien.

This is a bad workflow because now only people with write access to the npm repo can develop on any app that will require a change to the mock and push the RC. This is a clear move backward.

This revision now requires changes to proceed.Mar 4 2024, 09:02

Modify templates to conditionally install mock-chronik-client deps if required

specify DEPENDS_MOCK_CHRONIK_CLIENT: "false" if unused

bytesofman retitled this revision from [mock-chronik-client] Do not run CI on impacted modules on repo changes to [mock-chronik-client] Install mock-chronik-client dependencies before running tests that require them.Mar 4 2024, 13:00
bytesofman edited the summary of this revision. (Show Details)
bytesofman edited the test plan for this revision. (Show Details)
contrib/teamcity/build-configurations.yml
735 ↗(On Diff #45884)

I was not able to get this to work setting the var as a bool (vs string "true" or "false"). Probably due to lack of expertise with bash but mb something to do with how this is passed to the script.

Fabien added inline comments.
contrib/teamcity/build-configurations.yml
74 ↗(On Diff #45884)
783 ↗(On Diff #45884)

You can just omit it instead of setting it to false (note this is required to be empty with the suggested -n change otherwise false will be interpreted as true)

This revision is now accepted and ready to land.Mar 5 2024, 09:10