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

NetworkTests: add options for test users to use gateway #1806

Merged
merged 5 commits into from
Feb 21, 2024

Conversation

BedrockSquirrel
Copy link
Collaborator

@BedrockSquirrel BedrockSquirrel commented Feb 20, 2024

Why this change is needed

The network tests are useful for end-to-end testing from the IDE against a local network or against the testnet environments.

But they could only test the direct to node 'AuthClient' connections and so they couldn't be used to debug or test GW issues.

What changes were made as part of this PR

  • Add WithGateway() option to the LocalDevNetwork so it starts a (debuggable) gateway process alongside the network nodes.
  • Add UseGateway boolean option to the CreateTestUser action to allow users to be created that would register and communicate with the gateway instead of directly. (Create User abstraction so both types of user can be used in all tests).
  • Fix issue in the gateway where it was expecting params key to always be set even for methods like eth_gasPrice that have no params. This is an issue because the geth client does not send that key for that method.

PR checks pre-merging

Please indicate below by ticking the checkbox that you have read and performed the required
PR checks

  • PR checks reviewed and performed

Copy link
Collaborator

@tudor-malene tudor-malene left a comment

Choose a reason for hiding this comment

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

lgtm.

As discussed, it would be good to change the User->Wallet relationship from is to has

Copy link
Contributor

@zkokelj zkokelj left a comment

Choose a reason for hiding this comment

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

LGTM, nice job.. very helpful test!👍

@BedrockSquirrel BedrockSquirrel merged commit 25f4f5b into main Feb 21, 2024
2 checks passed
@BedrockSquirrel BedrockSquirrel deleted the matt/gw-network-tests branch February 21, 2024 10:07
zkokelj added a commit that referenced this pull request Feb 21, 2024
* fix cache for authenticated method calls

* fix mutex error

* NetworkTests: add options for test users to use gateway (#1806)

---------

Co-authored-by: Matt <[email protected]>
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.

3 participants