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

Client: Release Consistency/Functionality Checks #1636

Closed
holgerd77 opened this issue Jan 15, 2022 · 7 comments
Closed

Client: Release Consistency/Functionality Checks #1636

holgerd77 opened this issue Jan 15, 2022 · 7 comments

Comments

@holgerd77
Copy link
Member

holgerd77 commented Jan 15, 2022

Introduction

We are now getting into a mode where we are doing regular releases on the client, see #1635 for an example on the next iteration here. The client is a bit of a different beast than the other libraries and running tests is not enough to ensure everything is working properly in a live environment.

It would be good to somewhat agree and define a set of tests we would want to run before a client release which both remains practicable but also ensures that the basic functionality of the client works.

Maybe some (all?) items from this list can also be (semi-)automated in some way, e.g. with a script where one manually checks the log output afterwards (just a first idea, open for alternative ones).

Possible Tasks

Here is a wishlist what would be nice, feel free to add (if you have the permissions directly change in here) or update:

  • Run the client on all networks provided and see if a connection is possible
  • Start the RPC client and see if a request is possible
  • See if the shutdown works correctly
  • Let the client run for some substantial amount of time (> 1 hour) and see if the connection remains stable + sync continues
  • Short test if mining blocks works
  • Test if a (local) libp2p connection works

Guess that would already be most of the essential/core stuff. We can also start/limit on the easier things (network connections) and leave out the more difficult ones (long-running test, local interoperability tests).

@acolytec3
Copy link
Contributor

Start the RPC client and see if a request is possible

What kind of request are you thinking here, just any RPC request to verify that the RPC is up and running or something specific like eth_getBlockByHash?

@acolytec3
Copy link
Contributor

I think a lot of these can be automated (except for maybe the long run one).

@ryanio
Copy link
Contributor

ryanio commented Jan 15, 2022

Yeah sounds good, we already have a lot of these.

Run the client on all networks provided and see if a connection is possible

The only network I've found reliable enough to run on ci is goerli, the others (mainnet, ropsten, rinkeby) can take 5-20 minutes to find an available peer for a block request (sync).

I have a github actions matrix in workflows/client-build.yml that uses test/rpc/cli-sync.spec.ts to sync and execute a first block. I think adding sepolia would be interesting to try.

We could have a separate test here that runs on label and/or release that gives ample time for the more important set of networks. Ones that have dns network routing should be more reliable to find available peers.

Start the RPC client and see if a request is possible

Yes @acolytec3 has added test/cli/cli-rpc.spec.ts 👍

See if the shutdown works correctly
Let the client run for some substantial amount of time (> 1 hour) and see if the connection remains stable + sync continues

👍

Short test if mining blocks works

I have mining both PoW and PoA blocks in test/miner/miner.spec.test

Test if a (local) libp2p connection works

That's a good one. a few short tests for the libp2p connection would be nice, i don't think we have anything e2e like that. another way might be to create some kind of example that's runnable in ci to establish a connection and maybe do something like sync one block. (this might be prohibited by port access in ci though, not sure as I've never tried, I could imagine it's locked down and not in any host mode.)

@holgerd77
Copy link
Member Author

The only network I've found reliable enough to run on ci is goerli, the others (mainnet, ropsten, rinkeby) can take 5-20 minutes to find an available peer for a block request (sync).

That might actually be outdated and we might want to update here? 🤔 Just tested on mainnet and rinkeby and got reliable connections repeatedly in < 20 seconds (each time deleting the data directory).

I guess we can forget a bit about ropsten since being retired soon, - yes - sepolia would be a nice addition though. Maybe we do this 2-step though and skip sepolia for now if still not completely settled and reliable.

What kind of request are you thinking here, just any RPC request to verify that the RPC is up and running or something specific like eth_getBlockByHash

Yes @acolytec3 has added test/cli/cli-rpc.spec.ts 👍

Ah, cool, sorry, I wasn't aware of this test any more. @acolytec3 yes, for this test I would personally say only one request is enough. The general functionality would then be tested by the normal unit tests.

See if the shutdown works correctly

Is this somewhat implicitly tested by e.g. the CLI tests to run a network (Goerli e.g.)? I would cautiously guess so since otherwise the test run wouldn't come to a halt? 🤔 Or am I missing some aspect here (which should be tested)?

Ah, I've forgotten about an important one:

  • Tip of the chain behaviour

@acolytec3
Copy link
Contributor

Is this somewhat implicitly tested by e.g. the CLI tests to run a network (Goerli e.g.)? I would cautiously guess so since otherwise the test run wouldn't come to a halt? 🤔 Or am I missing some aspect here (which should be tested)?

I think you're right but we can test it explicitly too. I'll add a test to my cli test and that will cover it

@holgerd77
Copy link
Member Author

I think you're right but we can test it explicitly too. I'll add a test to my cli test and that will cover it

Cool, yeah, that's definitely better if done explicitly.

@holgerd77
Copy link
Member Author

While still room for continued evolution the base issue has been sufficiently covered over time, latest with the addition of CLI option tests in #2916 .

Will close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants