Tenv update #17195
Tenv update #17195
Conversation
WalkthroughThis pull request updates the Docker image tag for the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (12)
⏰ Context from checks skipped due to timeout of 90000ms (13)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/suite-desktop-core/e2e/tests/onboarding/t2t1/t2t1-create-wallet.test.ts (1)
45-45: 🛠️ Refactor suggestionAdd more assertions for comprehensive validation.
The test currently only verifies PIN button visibility. Consider adding assertions to validate:
- Successful creation of 3 shares
- Correct threshold configuration (2-of-3)
- Complete backup process verification
Example assertions to add:
await expect(page.getByTestId('@backup/shares-count')).toHaveText('3'); await expect(page.getByTestId('@backup/threshold-count')).toHaveText('2'); await expect(page.getByTestId('@backup/success-screen')).toBeVisible();
🧹 Nitpick comments (1)
packages/suite-desktop-core/e2e/tests/onboarding/t2t1/t2t1-create-wallet.test.ts (1)
16-46: Consider enhancing test coverage.While the test covers the happy path for Shamir backup creation with a 3-of-2 scheme, consider adding tests for:
- Edge cases (minimum/maximum shares/threshold)
- Error scenarios (user cancellation, device disconnection)
- Recovery verification
Would you like me to help create additional test cases to improve coverage?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/test-trezor-user-env-link-e2e.yml(1 hunks)docker/docker-compose.connect-popup-ci.yml(1 hunks)docker/docker-compose.connect-popup-test.yml(1 hunks)docker/docker-compose.connect-test.yml(1 hunks)docker/docker-compose.connect-webextension-test.yml(1 hunks)docker/docker-compose.suite-ci-pw.yml(1 hunks)docker/docker-compose.suite-ci.yml(1 hunks)docker/docker-compose.suite-desktop-ci.yml(1 hunks)docker/docker-compose.suite-dev.yml(1 hunks)docker/docker-compose.suite-native-ci.yml(1 hunks)docker/docker-compose.suite-test.yml(1 hunks)docker/docker-compose.transport-test-ci.yml(1 hunks)packages/suite-desktop-core/e2e/tests/onboarding/t2t1/t2t1-create-wallet.test.ts(1 hunks)submodules/trezor-common(1 hunks)
✅ Files skipped from review due to trivial changes (11)
- docker/docker-compose.connect-test.yml
- docker/docker-compose.suite-desktop-ci.yml
- submodules/trezor-common
- docker/docker-compose.suite-native-ci.yml
- docker/docker-compose.suite-test.yml
- docker/docker-compose.suite-ci.yml
- docker/docker-compose.suite-ci-pw.yml
- docker/docker-compose.connect-webextension-test.yml
- docker/docker-compose.connect-popup-ci.yml
- docker/docker-compose.connect-popup-test.yml
- docker/docker-compose.transport-test-ci.yml
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Other Checks
- GitHub Check: Releases revision Checks
- GitHub Check: Type Checking
- GitHub Check: Build libs for publishing
- GitHub Check: Linting and formatting
- GitHub Check: Unit Tests
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: build-web
- GitHub Check: e2e-test-trezor-user-env-link
- GitHub Check: Analyze with CodeQL (javascript)
🔇 Additional comments (3)
packages/suite-desktop-core/e2e/tests/onboarding/t2t1/t2t1-create-wallet.test.ts (1)
4-14:❓ Verification inconclusive
LGTM! Test suite re-enabled with proper setup.
The test suite is now active and correctly configured with:
- Latest T2T1 emulator
- Clean device state (wiped before tests)
- Appropriate test environment setup
Please verify the compatibility with the new tenv image by running:
🏁 Script executed:
#!/bin/bash # Description: Check if the new tenv image supports T2T1 emulator and Shamir backup # Get the tenv image version from docker-compose files tenv_image=$(fd -t f docker-compose.yml -x grep -l trezor-user-env-unix {} \; | xargs cat | grep "trezor-user-env-unix:" -A 1 | grep image | head -1) # Check if the image exists and can be pulled docker pull $tenv_image # Verify T2T1 emulator and Shamir backup support docker run --rm $tenv_image trezorctl versionLength of output: 402
Attention: Manual Verification Required
The test suite re-enabling looks properly set up (with the latest T2T1 emulator configuration, clean device state, and correct environment settings). However, the automated verification of compatibility with the new tenv image failed because the requireddockercommand wasn’t available (as shown in the error output).
- Action Required: Please manually verify the tenv image compatibility to ensure it supports the T2T1 emulator and Shamir backup test, using the appropriate environment where
dockeris installed..github/workflows/test-trezor-user-env-link-e2e.yml (1)
39-39: Confirm Docker Image Tag Update.
The Docker run command now uses the updated image tagdadc035aa0c121140dba0991ec6abdb786e18406as expected from the tenv update. Ensure this tag is consistent with other configuration files and reflects the correct build from the related trezor-user-env PR.docker/docker-compose.suite-dev.yml (1)
7-7: Verify Updated Docker Image in Service Definition.
Thetrezor-user-env-unixservice now references the new image tagdadc035aa0c121140dba0991ec6abdb786e18406. This update aligns with the tenv image update detailed in the PR objectives. Confirm across all environments that this change will not cause compatibility issues with other services.
|
changes in submodules, expected? |
No, my bad, force-pushed. Thanks for noticing |
Using new tenv image built from trezor/trezor-user-env#290
Enabling the Shamir T2T1 backup test again - it should be fixed