Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transport - fixes and more testing scenarios #13493

Merged
merged 6 commits into from
Aug 13, 2024
Merged

Conversation

mroz22
Copy link
Contributor

@mroz22 mroz22 commented Jul 24, 2024

🐰 🕳️

Commits

  • ca388c5 - to be able to use the already existing test for testing node-bridge too, we need to move the tests to transport-bridge package. The reason is that now these tests must import from both transport and transport-bridge package and transport-bridge is not dependency of transport package.
  • 9d9e904 - cleanup in sessions background.
  • 1fe634e - tests got adapted so that we can run them in 4 different setups (old/new, hw/emu). useful for detecting differences between implementations (there should be none) but the fact is that there are some at the moment, all of them documented here https://github.com/trezor/trezor-suite/blob/transport-meow/packages/transport-bridge/e2e/expect.ts#L1-L2
  • d37cc1c - CI now runs both emulator variants of the transport-birdge tests. I also made some changes in setup to make the tests faster. I believe that running jest test runner in a dedicated container is not necessary since they may be easily run in the base container. It saves maybe 2 minutes or so.
  • 606203b - this only renames events.test.ts to multi-client.test.ts because this test is indeed focusing on how TransportBridge class is behaving in this scenario.
  • 0eeb8e6 - this commit adds a new test documenting incorrect behavior of node-bridge in cases of concurrent enumeration. I believe that this issue comes from node USB lib or possibly from the underlying os USB drivers.
  • 582b299 - solution for the issue documented in the prev commit. we simply run enumeration in sequence one by one with the help of sessions background.
  • 7473ced - disabling unrelated test, issue created connect-popup: tests setup - changes in sharedworker impossible #13762
  • 83873b8 - /read endpoint should not parse body as JSON as added in Chore/transport bridge types #13592.

Follow-ups:

Issues:

@mroz22 mroz22 force-pushed the transport-meow branch 20 times, most recently from 7051698 to 09ce278 Compare July 30, 2024 07:13
@mroz22 mroz22 mentioned this pull request Jul 30, 2024
@mroz22 mroz22 force-pushed the transport-meow branch 9 times, most recently from c918082 to 9d1882e Compare August 2, 2024 09:20
@mroz22 mroz22 force-pushed the transport-meow branch 4 times, most recently from 537af70 to 0b2c9cb Compare August 7, 2024 09:56
@mroz22 mroz22 changed the title [wip] transport transport - fixes and more testing scenarios Aug 12, 2024
@mroz22 mroz22 marked this pull request as ready for review August 12, 2024 09:07
@mroz22 mroz22 force-pushed the transport-meow branch 6 times, most recently from 1fb3840 to c8f1c7f Compare August 12, 2024 13:59
Copy link
Contributor

@szymonlesisz szymonlesisz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all 4 tests are passing on my machine

@szymonlesisz szymonlesisz merged commit 94d1a26 into develop Aug 13, 2024
81 of 83 checks passed
@szymonlesisz szymonlesisz deleted the transport-meow branch August 13, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants