Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

add simnet tests#9222

Merged
TriplEight merged 4 commits intomasterfrom
simnet-tests
Jul 2, 2021
Merged

add simnet tests#9222
TriplEight merged 4 commits intomasterfrom
simnet-tests

Conversation

@radupopa2010
Copy link
Copy Markdown
Contributor

@radupopa2010 radupopa2010 commented Jun 28, 2021

This PR adds directory which will hold high level integration tests that will run with the simnet set of tools.

At the moment those kind of tests are maintained by the simnet team in simnet repo which is hosted just on gitlab.
This was done in this manner while we developed working version of tools to spawn a simulation network based on a docker image and run tests against this network.

There will be a follow up PR to edit the Ci file of this project, but I need those files hosted in this repo before.

We are still developing simnet and we are thinking how to make it simpler to use.

By moving simnet tests into this repository, the expectations are that developers would be able to maintain and create new tests.
Off course simnet team will help explaining how things work and how to use simnet tools

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Jun 28, 2021
@radupopa2010 radupopa2010 requested a review from TriplEight June 28, 2021 15:50
@radupopa2010 radupopa2010 added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jun 28, 2021
@radupopa2010 radupopa2010 requested review from bkchr and grbIzl June 28, 2021 15:55
@grbIzl
Copy link
Copy Markdown
Contributor

grbIzl commented Jun 29, 2021

Please remove simnet_tests/configs/local_testnet_with_logs.toml It was my debug for connection problems. Should be cleaned on Gitlab as well

forward_port
run_test


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this script a minimum that's required to launch simnet? In order to lower the maintenance, is it possible to move its' parts to simnet repo?

Copy link
Copy Markdown
Contributor Author

@radupopa2010 radupopa2010 Jun 29, 2021

Choose a reason for hiding this comment

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

This script sometimes is used for the setup of the tests, when we have no config options in the config files.

Each type of test, substrate or parachains has it's own version of run_test.sh.
Fedor was working on another version of this script to spawn a huge parachains network with different setup then the tests we already run

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it combines three functions then: configure, launch and add some testing features. While it should only pass the payload to simnet. Launching mechanism should be on simnet side, testing should happen in test files.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@TriplEight can you please elaborate? Right now this script is the entry point for the set of tests. Generally it has the following sections:

  • Collect required variables
  • Spawn the chain, using config files
  • Perform additional setup for the chain (things, that cannot be done inside configuration)
  • Run set of tests

So this script binds configuration and testing logic into one "entity", and CI runs this "entity". What parts do you suggest to move?

Copy link
Copy Markdown
Contributor

@TriplEight TriplEight Jun 30, 2021

Choose a reason for hiding this comment

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

As a tool, Simnet should have an entrypoint, the user should pass the requirements to the entrypoint, not triggering it in three steps. I suggest on the side of CI we store only the configs and tests, collect them and send to simnet.
And Simnet will

  • accept tests and configs
  • spawn the chain (setup)
  • initiate the tests
  • teardown

"Perform additional setup for the chain (things, that cannot be done inside configuration)" looks like another (5th or 6th) config entity, we should have less of them, can it go into the Simnet code with certain API points or feature flags?

I'm not suggesting doing everything in this PR, ofc.

Copy link
Copy Markdown
Contributor Author

@radupopa2010 radupopa2010 Jul 2, 2021

Choose a reason for hiding this comment

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

I agree with you Denis, this is the ideal case. In order to do this we need to do a bit more development and testing.
This will delay a the move of tests into the substrate repo. We need to think about the trade-offs.
For now we can live with the current state and do further iterations to improve the configs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah, just note this somewhere, please. This PR can be merged.

@radupopa2010 radupopa2010 requested a review from TriplEight June 29, 2021 16:36
@TriplEight TriplEight merged commit 6b7ab81 into master Jul 2, 2021
@TriplEight TriplEight deleted the simnet-tests branch July 2, 2021 10:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants