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

Introduce test runner biolerplate #2609

Merged
merged 81 commits into from
Jul 13, 2021
Merged

Introduce test runner biolerplate #2609

merged 81 commits into from
Jul 13, 2021

Conversation

seunlanlege
Copy link
Contributor

@seunlanlege seunlanlege commented Mar 11, 2021 •
edited
Loading

@seunlanlege seunlanlege added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). 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 Mar 11, 2021
@ordian ordian added A0-please_review Pull request needs code review. A4-companion A PR that needs a companion PR to merge in parallel for one of its downstream dependencies. and removed A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). labels Mar 11, 2021
@ordian
Update node/test/runtime/Cargo.toml
f1a4d69 
Co-authored-by: Andronik Ordian <write@reusable.software>
@seunlanlege seunlanlege added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. label Mar 23, 2021
@seunlanlege
Copy link
Contributor Author

@ordian @kianenigma @bkchr could you please take a look at this?

ordian
ordian reviewed Mar 25, 2021
View reviewed changes
Cargo.lock Outdated Show resolved Hide resolved
@kianenigma kianenigma self-requested a review March 30, 2021 05:03
ordian
ordian reviewed Jul 9, 2021
View reviewed changes
Cargo.lock Outdated Show resolved Hide resolved
@ordian ordian dismissed their stale review July 12, 2021 13:37

stale

@ordian
Copy link
Member

ordian commented Jul 12, 2021

every new file uses spaces instead of tabs, please fix that as well as cargo update reversion

ordian
ordian reviewed Jul 12, 2021
View reviewed changes
runtime/polkadot/src/lib.rs Outdated Show resolved Hide resolved
@seunlanlege
Copy link
Contributor Author

every new file uses spaces instead of tabs, please fix that as well as cargo update reversion

damn, vscode strikes again

@ordian ordian requested a review from drahnr July 13, 2021 10:39
@ordian ordian added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. A4-companion A PR that needs a companion PR to merge in parallel for one of its downstream dependencies. labels Jul 13, 2021
drahnr
drahnr reviewed Jul 13, 2021
View reviewed changes
node/test/polkadot-simnet/common/src/lib.rs
let mut tokio_runtime = build_runtime()?;
let task_executor = task_executor(tokio_runtime.handle().clone());
// parse cli args
let cmd = <polkadot_cli::Cli as StructOpt>::from_args();
Copy link
Contributor

@drahnr drahnr Jul 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

nit: There should be variant of this run that allows passing the Cli args explicitly rather than expecting them to present in the environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, interesting but what would be the use case?

Copy link
Contributor

@drahnr drahnr Jul 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

My idea was to also use this for small scale integration test level networks, launching 3 nodes i.e. which should be smth a regular dev laptop can handle. At least that would be my prime use case, in combination with some adapted behavior based on malus.

Given that, the boilerplate code should be unified so it lives in one place so we avoid future pain applying changes for the same things in 3 places.

This is slightly in/out-of-scope with this PR, so this is optional for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could be added in a future PR, when the need for the functionality arises

drahnr
drahnr approved these changes Jul 13, 2021
View reviewed changes
Copy link
Contributor

@drahnr drahnr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Recently I introduced some resolution issue for sp-core and iiuc this would re-introduce that problem, iff @ordian was aware of that and accounted for in his review,
LGTM!

@seunlanlege seunlanlege merged commit 08e47ab into master Jul 13, 2021
@seunlanlege seunlanlege deleted the substrate-test-runner branch July 13, 2021 15:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ordian ordian ordian approved these changes

@drahnr drahnr drahnr approved these changes

@gilescope gilescope gilescope approved these changes

@kianenigma kianenigma Awaiting requested review from kianenigma

Assignees
No one assigned
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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@seunlanlege @ordian @drahnr @gilescope @gavofyork @paritytech-ci @radupopa2010