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

Refactor/split server's user tests #487

Open
ameba23 opened this issue Nov 9, 2023 · 2 comments
Open

Refactor/split server's user tests #487

ameba23 opened this issue Nov 9, 2023 · 2 comments
Labels

Comments

@ameba23
Copy link
Contributor

ameba23 commented Nov 9, 2023

@HCastano noted in this PR comment that the sign_tx_no_chain test is very long and has both happy case and negative test, and could maybe be split up: #475 (comment)

I think this is also true of some of the other tests in https://github.com/entropyxyz/entropy-core/blob/master/crypto/server/src/user/tests.rs - they are a bit difficult to maintain and could be refactored.

Why is this issue relevant?

We want to have tests make it easy to see what the problem is when they fail, and are easy to maintain when we want to make changes

What steps are required to resolve this?

Refactor the tests a bit

Does this change the spec? HTTP, extrinsic, or storage? Is it breaking? Clearly describe the new interface.

Not breaking

@HCastano
Copy link
Collaborator

HCastano commented Nov 9, 2023

I've been looking into this recently and I believe the crux of the issue here is that a
lot of the tests depend on a running Substrate node, and this setup is quite expensive
(it takes about 30s for a debug node and 10s for a release node on my end to get the
process to a point where we can run tests against it).

Instead of spinning up multiple nodes for multiple tests it's quicker to do the setup
once and just write a ton of tests under the scope of a single "test".

Some ways we could potentially split the tests up:

  • Have a way to run tests without spinning up a Substrate node
    • In practice this means refactoring the tests in such a way that they don't need to
      actually run against a node unless it's strictly necessary
  • Have a single global node process spun up at the beginning of each run and run all the
    tests against that
    • This assumes that the state of the node doesn't really matter across tests, which may
      be the case in individual modules but maybe not across them
  • Have a more lightweight way to spin up node processes
    • I haven't found a way to do this at the moment

@HCastano
Copy link
Collaborator

HCastano commented Sep 9, 2024

Some of this was done in #1026, but the whole test could still be split out more.

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

No branches or pull requests

2 participants