From 728d6e0708e29b214f1ac0f7dd9463179c46a8c8 Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Fri, 15 Nov 2024 11:09:48 +0100 Subject: [PATCH] test(connect): specify transport --- .../template-connect-test-params.yml | 2 + .github/workflows/test-connect.yml | 29 +- docker/docker-connect-test.sh | 10 +- packages/connect/e2e/common.setup.ts | 5 +- .../connect/e2e/tests/device/cancel.test.ts | 278 +++++++++--------- .../transport-test/e2e/bridge/controller.ts | 3 +- packages/trezor-user-env-link/src/api.ts | 4 +- scripts/ci/connect-test-matrix-generator.js | 2 +- 8 files changed, 180 insertions(+), 153 deletions(-) diff --git a/.github/workflows/template-connect-test-params.yml b/.github/workflows/template-connect-test-params.yml index c8c3c25ea4f..b43581b0180 100644 --- a/.github/workflows/template-connect-test-params.yml +++ b/.github/workflows/template-connect-test-params.yml @@ -96,4 +96,6 @@ jobs: run: echo "ADDITIONAL_ARGS=$ADDITIONAL_ARGS -i ${{ inputs.includeFilter }}" >> "$GITHUB_ENV" - if: ${{ inputs.testRandomizedOrder }} run: echo "ADDITIONAL_ARGS=$ADDITIONAL_ARGS -r" >> "$GITHUB_ENV" + - if: ${{ inputs.transport }} + run: echo "ADDITIONAL_ARGS=$ADDITIONAL_ARGS -t ${{ inputs.transport }}" >> "$GITHUB_ENV" - run: './docker/docker-connect-test.sh ${{ inputs.testEnv }} -p "${{ inputs.testPattern }}" -f "${{ inputs.testsFirmware }}" $ADDITIONAL_ARGS' diff --git a/.github/workflows/test-connect.yml b/.github/workflows/test-connect.yml index 001be06a531..22646b92603 100644 --- a/.github/workflows/test-connect.yml +++ b/.github/workflows/test-connect.yml @@ -70,21 +70,26 @@ jobs: dailyMatrix: ${{ steps.set-matrix-daily.outputs.dailyMatrix }} otherDevicesMatrix: ${{ steps.set-matrix-other-devices.outputs.otherDevicesMatrix }} allFwsMatrix: ${{ steps.set-matrix-all-firmwares.outputs.allFwsMatrix }} + allTransportsMatrix: ${{ steps.set-matrix-all-transports.outputs.allTransportsMatrix }} steps: - name: Checkout repository uses: actions/checkout@v4 - name: Set daily matrix id: set-matrix-daily - run: echo "dailyMatrix=$(node ./scripts/ci/connect-test-matrix-generator.js --model=T2T1 --firmware=2-latest --env=all --groups=api,api-flaky --cache_tx=true --transport=Bridge)" >> $GITHUB_OUTPUT + run: echo "dailyMatrix=$(node ./scripts/ci/connect-test-matrix-generator.js --model=T2T1 --firmware=2-latest --env=all --groups=api,api-flaky --cache_tx=true --transport=2.0.33)" >> $GITHUB_OUTPUT - name: Set all firmwares matrix id: set-matrix-all-firmwares - run: echo "allFwsMatrix=$(node ./scripts/ci/connect-test-matrix-generator.js --model=T2T1 --firmware=all --env=all --groups=all --cache_tx=false --transport=Bridge)" >> $GITHUB_OUTPUT + run: echo "allFwsMatrix=$(node ./scripts/ci/connect-test-matrix-generator.js --model=T2T1 --firmware=all --env=all --groups=all --cache_tx=false --transport=2.0.33)" >> $GITHUB_OUTPUT - name: Set other devices matrix id: set-matrix-other-devices - run: echo "otherDevicesMatrix=$(node ./scripts/ci/connect-test-matrix-generator.js --model=all --firmware=2-main --env=node --groups=api --cache_tx=true --transport=Bridge)" >> $GITHUB_OUTPUT + run: echo "otherDevicesMatrix=$(node ./scripts/ci/connect-test-matrix-generator.js --model=all --firmware=2-main --env=node --groups=api --cache_tx=true --transport=2.0.33)" >> $GITHUB_OUTPUT + + - name: Set all transports matrix + id: set-matrix-all-transports + run: echo "allTransportsMatrix=$(node ./scripts/ci/connect-test-matrix-generator.js --model=T2T1 --firmware=2-latest --env=node --groups=api --cache_tx=true --transport=2.0.32,2.0.33)" >> $GITHUB_OUTPUT PR-check: needs: [build, set-matrix] @@ -158,3 +163,21 @@ jobs: strategy: fail-fast: false matrix: ${{ fromJson(needs.set-matrix.outputs.otherDevicesMatrix) }} + + all-transports: + needs: [build, set-matrix] + name: all-transports-api ${{ matrix.key }} {{ matrix.transport }} + if: (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') && github.repository == 'trezor/trezor-suite' + uses: ./.github/workflows/template-connect-test-params.yml + with: + testPattern: ${{ matrix.groups.pattern }} + includeFilter: ${{ matrix.groups.includeFilter }} + testsFirmware: ${{ matrix.firmware }} + testDescription: ${{ matrix.transport }} + cache_tx: ${{ matrix.cache_tx }} + transport: ${{ matrix.transport }} + testEnv: ${{ matrix.env }} + testFirmwareModel: ${{ matrix.model }} + strategy: + fail-fast: false + matrix: ${{ fromJson(needs.set-matrix.outputs.allTransportsMatrix) }} diff --git a/docker/docker-connect-test.sh b/docker/docker-connect-test.sh index 20c6c278d6e..77306bddf5a 100755 --- a/docker/docker-connect-test.sh +++ b/docker/docker-connect-test.sh @@ -40,6 +40,7 @@ show_usage() { echo " -u Firmware url" echo " -m Firmware model, example: R'" echo " -r Randomize test order (node only)" + echo " -t Specify transport / bridge version" } # default options @@ -53,10 +54,12 @@ USE_WS_CACHE=true PATTERN="" FIRMWARE_MODEL="" RANDOMIZE=false +TRANSPORT="2.0.33" +# echo $OPTARG # user options OPTIND=2 -while getopts ":p:i:e:f:u:m:hdcr" opt; do +while getopts ":p:i:e:f:u:m:t:hdcr" opt; do case $opt in d) DOCKER=false @@ -86,6 +89,9 @@ while getopts ":p:i:e:f:u:m:hdcr" opt; do r) RANDOMIZE=true ;; + t) + TRANSPORT=$OPTARG + ;; h) # Script usage show_usage exit 0 @@ -115,6 +121,7 @@ export TESTS_SCRIPT=$SCRIPT export TESTS_FIRMWARE_URL=$FIRMWARE_URL export TESTS_FIRMWARE_MODEL=$FIRMWARE_MODEL export TESTS_RANDOM=$RANDOMIZE +export TESTS_TRANSPORT=$TRANSPORT runDocker() { docker compose -f ./docker/docker-compose.connect-test.yml up -d @@ -132,6 +139,7 @@ run() { echo " TxCache: ${USE_TX_CACHE}" echo " WsCache: ${USE_WS_CACHE}" echo " Random: ${RANDOMIZE}" + echo " Transport: ${TRANSPORT}" if [ $DOCKER = true ]; then runDocker diff --git a/packages/connect/e2e/common.setup.ts b/packages/connect/e2e/common.setup.ts index 89f3a425ffc..59545473e64 100644 --- a/packages/connect/e2e/common.setup.ts +++ b/packages/connect/e2e/common.setup.ts @@ -83,7 +83,10 @@ export const setup = async ( TrezorUserEnvLink.state = options; // after all is done, start bridge again - await TrezorUserEnvLink.startBridge(); + await TrezorUserEnvLink.startBridge( + // @ts-expect-error + process.env.TESTS_TRANSPORT, + ); }; export const initTrezorConnect = async ( diff --git a/packages/connect/e2e/tests/device/cancel.test.ts b/packages/connect/e2e/tests/device/cancel.test.ts index 1cd9ed9d9d9..cc7dc7d329f 100644 --- a/packages/connect/e2e/tests/device/cancel.test.ts +++ b/packages/connect/e2e/tests/device/cancel.test.ts @@ -44,18 +44,15 @@ const assertGetAddressWorks = async () => { describe('TrezorConnect.cancel', () => { const controller = getController(); - const setupTest = async ({ - setupParams, - bridgeVersion, - }: { - setupParams: SetupEmu & StartEmu; - bridgeVersion: string; - }) => { + const setupTest = async ({ setupParams }: { setupParams: SetupEmu & StartEmu }) => { await controller.stopBridge(); await controller.stopEmu(); await controller.startEmu({ wipe: true, ...setupParams }); await controller.setupEmu(setupParams); - await controller.startBridge(bridgeVersion); + await controller.startBridge( + // @ts-expect-error + process.env.TESTS_TRANSPORT, + ); await initTrezorConnect(controller, { debug: true }); }; @@ -69,165 +66,156 @@ describe('TrezorConnect.cancel', () => { controller.dispose(); }); - [ - // '2.0.27', // todo: 2.0.27 is removed from trezor-user-env - '2.0.33', - // '3.0.0' // todo: add support in trezor-user-env - ].forEach(bridgeVersion => { - describe(`Bridge ${bridgeVersion}`, () => { - afterEach(async () => { - await TrezorConnect.dispose(); - await controller.stopEmu(); - await controller.stopBridge(); - }); + afterEach(async () => { + await TrezorConnect.dispose(); + await controller.stopEmu(); + await controller.stopBridge(); + }); + + // the goal is to run this test couple of times to uncover possible race conditions/flakiness + it(`GetAddress - ButtonRequest_Address - Cancel `, async () => { + await setupTest({ + setupParams: {}, + }); - // the goal is to run this test couple of times to uncover possible race conditions/flakiness - it(`GetAddress - ButtonRequest_Address - Cancel `, async () => { - await setupTest({ - setupParams: {}, - bridgeVersion, - }); - - TrezorConnect.removeAllListeners(); - const getAddressCall = getAddress(true); - await new Promise(resolve => { - TrezorConnect.on('button', event => { - if (event.code === 'ButtonRequest_Address') { - resolve(); - } - }); - }); - - TrezorConnect.cancel('Cancel reason'); - - const response = await getAddressCall; - - expect(response).toMatchObject({ - success: false, - payload: { - error: 'Cancel reason', - code: 'Method_Cancel', - }, - }); - - // TODO: here I would like to continue and validate that I can communicate after a cancelled call - // await assertGetAddressWorks(); - - // but this sometimes fails (nodejs) with, probably a race condition - // success: false, - // payload: { - // error: 'Initialize failed: Unexpected message, code: Failure_UnexpectedMessage', - // code: 'Device_InitializeFailed' - // } + TrezorConnect.removeAllListeners(); + const getAddressCall = getAddress(true); + await new Promise(resolve => { + TrezorConnect.on('button', event => { + if (event.code === 'ButtonRequest_Address') { + resolve(); + } }); + }); - it('Synchronous Cancel', async () => { - await setupTest({ - setupParams: {}, - bridgeVersion, - }); + TrezorConnect.cancel('Cancel reason'); - TrezorConnect.removeAllListeners(); - const getAddressCall = getAddress(true); + const response = await getAddressCall; - // almost synchronous, TODO: core methodSynchronize race-condition in nodejs (works in web) - await new Promise(resolve => setTimeout(resolve, 1)); + expect(response).toMatchObject({ + success: false, + payload: { + error: 'Cancel reason', + code: 'Method_Cancel', + }, + }); - TrezorConnect.cancel('Cancel reason'); + // TODO: here I would like to continue and validate that I can communicate after a cancelled call + // await assertGetAddressWorks(); - const response = await getAddressCall; + // but this sometimes fails (nodejs) with, probably a race condition + // success: false, + // payload: { + // error: 'Initialize failed: Unexpected message, code: Failure_UnexpectedMessage', + // code: 'Device_InitializeFailed' + // } + }); - expect(response).toMatchObject({ - success: false, - payload: { - error: 'Cancel reason', - }, - }); + it('Synchronous Cancel', async () => { + await setupTest({ + setupParams: {}, + }); - // todo: this test doesn't work properly - // getAddressCall is rejected while device is acquiring workflow continues... - // assertion will result with "device call in progress" - // await assertGetAddressWorks(); - }); + TrezorConnect.removeAllListeners(); + const getAddressCall = getAddress(true); - it('Passphrase request - Cancel', async () => { - await setupTest({ - setupParams: { - passphrase_protection: true, - }, - bridgeVersion, - }); + // almost synchronous, TODO: core methodSynchronize race-condition in nodejs (works in web) + await new Promise(resolve => setTimeout(resolve, 1)); - const getAddressCall = getAddress(true); - await new Promise(resolve => { - TrezorConnect.on('ui-request_passphrase', () => resolve()); - }); - TrezorConnect.cancel(); + TrezorConnect.cancel('Cancel reason'); - const response = await getAddressCall; + const response = await getAddressCall; - expect(response.success).toEqual(false); + expect(response).toMatchObject({ + success: false, + payload: { + error: 'Cancel reason', + }, + }); - await assertGetAddressWorks(); - }); + // todo: this test doesn't work properly + // getAddressCall is rejected while device is acquiring workflow continues... + // assertion will result with "device call in progress" + // await assertGetAddressWorks(); + }); - // todo: this should be conditionalTest(['2']) - it('Pin request - Cancel', async () => { - await setupTest({ - setupParams: { - version: '1-latest', - model: 'T1B1', - pin: '1234', - }, - bridgeVersion, - }); - // T1 needs to be restarted for settings to be applied (pin) - await controller.stopEmu(); - await controller.startEmu({ version: '1-latest', model: 'T1B1' }); - - const pinPromise = new Promise(resolve => { - TrezorConnect.on('ui-request_pin', () => { - resolve(); - }); - }); - const getAddressCall = getAddress(true); - await pinPromise; - TrezorConnect.cancel(); - - const response = await getAddressCall; - - expect(response.success).toEqual(false); - - // assertGetAddressWorks will not work without providing pin - const feat = await TrezorConnect.getFeatures(); - expect(feat.payload).toMatchObject({ initialized: true }); - }); + it('Passphrase request - Cancel', async () => { + await setupTest({ + setupParams: { + passphrase_protection: true, + }, + }); - // todo: this should be conditionalTest(['2']) - it('Word request - Cancel', async () => { - await controller.startEmu({ version: '1-latest', model: 'T1B1', wipe: true }); - await controller.startBridge(bridgeVersion); - await initTrezorConnect(controller); + const getAddressCall = getAddress(true); + await new Promise(resolve => { + TrezorConnect.on('ui-request_passphrase', () => resolve()); + }); + TrezorConnect.cancel(); - const wordPromise = new Promise(resolve => { - TrezorConnect.on('ui-request_word', () => resolve()); - }); - const recoveryDeviceCall = TrezorConnect.recoveryDevice({ - passphrase_protection: false, - pin_protection: false, - }); + const response = await getAddressCall; - await wordPromise; - TrezorConnect.cancel(); + expect(response.success).toEqual(false); - const response = await recoveryDeviceCall; + await assertGetAddressWorks(); + }); - expect(response.success).toEqual(false); + // todo: this should be conditionalTest(['2']) + it('Pin request - Cancel', async () => { + await setupTest({ + setupParams: { + version: '1-latest', + model: 'T1B1', + pin: '1234', + }, + }); + // T1 needs to be restarted for settings to be applied (pin) + await controller.stopEmu(); + await controller.startEmu({ version: '1-latest', model: 'T1B1' }); - // assertGetAddressWorks will not work here, device is not initialized - const feat = await TrezorConnect.getFeatures(); - expect(feat.payload).toMatchObject({ initialized: false }); + const pinPromise = new Promise(resolve => { + TrezorConnect.on('ui-request_pin', () => { + resolve(); }); }); + const getAddressCall = getAddress(true); + await pinPromise; + TrezorConnect.cancel(); + + const response = await getAddressCall; + + expect(response.success).toEqual(false); + + // assertGetAddressWorks will not work without providing pin + const feat = await TrezorConnect.getFeatures(); + expect(feat.payload).toMatchObject({ initialized: true }); + }); + + // todo: this should be conditionalTest(['2']) + it('Word request - Cancel', async () => { + await controller.startEmu({ version: '1-latest', model: 'T1B1', wipe: true }); + await controller.startBridge( + // @ts-expect-error + process.env.TESTS_TRANSPORT, + ); + await initTrezorConnect(controller); + + const wordPromise = new Promise(resolve => { + TrezorConnect.on('ui-request_word', () => resolve()); + }); + const recoveryDeviceCall = TrezorConnect.recoveryDevice({ + passphrase_protection: false, + pin_protection: false, + }); + + await wordPromise; + TrezorConnect.cancel(); + + const response = await recoveryDeviceCall; + + expect(response.success).toEqual(false); + + // assertGetAddressWorks will not work here, device is not initialized + const feat = await TrezorConnect.getFeatures(); + expect(feat.payload).toMatchObject({ initialized: false }); }); }); diff --git a/packages/transport-test/e2e/bridge/controller.ts b/packages/transport-test/e2e/bridge/controller.ts index f7f930a4593..8a63d902c3c 100644 --- a/packages/transport-test/e2e/bridge/controller.ts +++ b/packages/transport-test/e2e/bridge/controller.ts @@ -55,7 +55,8 @@ class Controller extends TrezorUserEnvLinkClass { this.startBridge = !env.USE_HW && !env.USE_NODE_BRIDGE - ? (version?: string) => this.originalApi.startBridge(version) + ? (version?: Parameters[0]) => + this.originalApi.startBridge(version) : env.USE_NODE_BRIDGE ? async () => { this.nodeBridge = new TrezordNode({ diff --git a/packages/trezor-user-env-link/src/api.ts b/packages/trezor-user-env-link/src/api.ts index c2a9616b695..4af222590de 100644 --- a/packages/trezor-user-env-link/src/api.ts +++ b/packages/trezor-user-env-link/src/api.ts @@ -54,6 +54,8 @@ interface ReadAndConfirmShamirMnemonicEmu { threshold: number; } +type StartBridgeVersion = '2.0.32' | '2.0.33' | 'node-bridge'; + export const MNEMONICS = { mnemonic_all: 'all all all all all all all all all all all all', mnemonic_12: 'alcohol woman abuse must during monitor noble actual mixed trade anger aisle', @@ -160,7 +162,7 @@ export class TrezorUserEnvLinkClass extends TypedEmitter return null; } - async startBridge(version = DEFAULT_BRIDGE_VERSION) { + async startBridge(version: StartBridgeVersion = DEFAULT_BRIDGE_VERSION) { await this.client.send({ type: 'bridge-start', version }); return null; diff --git a/scripts/ci/connect-test-matrix-generator.js b/scripts/ci/connect-test-matrix-generator.js index 0e00096d930..a91a2c7e872 100644 --- a/scripts/ci/connect-test-matrix-generator.js +++ b/scripts/ci/connect-test-matrix-generator.js @@ -98,7 +98,7 @@ const inputs = [ }, { key: 'transport', - value: ['Bridge', 'NodeBridge'], + value: ['2.0.32', '2.0.33', 'node-bridge'], }, { key: 'groups',