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

PoC: Integration tests with generic fudge environment #1578

Closed
wants to merge 1 commit into from

Conversation

lemunozm
Copy link
Contributor

@lemunozm lemunozm commented Oct 2, 2023

Description

Proof of concept to convert the env fudge module of integration-test in a Runtime independent way. So after this, test cases will be based on <T: Config> where T is the current runtime.

@lemunozm lemunozm self-assigned this Oct 2, 2023

use crate::utils::{logs, time::START_DATE};

pub trait Config:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Common configuration between all our runtimes

Latest,
}

pub trait 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.

The environment wraps different fudge::companion structs to be used independently of the runtime

Comment on lines 33 to 47
#[fudge::companion]
pub struct AltairEnv {
#[fudge::relaychain]
pub relay: RelaychainBuilder<RelayBlock, RelayRtApi, RelayRt, RelayCidp, RelayDp>,
#[fudge::parachain(PARA_ID)]
pub parachain: ParachainBuilder<AltairBlock, AltairRtApi, AltairCidp, AltairDp>,
nonce_manager: Arc<Mutex<NonceManager>>,
pub events: Arc<Mutex<EventsStorage>>,
}

impl Environment for AltairEnv {
// TODO...
// implement non-default methods of Environment trait
}

impl Config for altair_runtime::Runtime {
//type RelayApi = kusama_runtime::RuntimeApi;
//type RelayRuntime = kusama_runtime::Runtime;

const KIND: RuntimeKind = RuntimeKind::Altair;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Each runtime should implement the Environment and the Config

@lemunozm
Copy link
Contributor Author

lemunozm commented Oct 2, 2023

@mustermeiszer maybe the ideas from this PoC can be used to build the Option A of your thoughts here

@lemunozm lemunozm force-pushed the integration-tests/generic-environment branch from ec5c302 to 94341f6 Compare October 2, 2023 09:28
@mustermeiszer
Copy link
Collaborator

I think my main problem is that test_with_all_runtimes assumes that the setup and so on is the same for all runtimes. Which I think is mostly true, but one essential breaking point are the differing AdminOrigin that we are using. Although, injecting via root should still work.

And tests would be generic over T: Config then?

@lemunozm
Copy link
Contributor Author

lemunozm commented Oct 2, 2023

Yes, the test would be generic over T: Config. I think procedural macros could be pretty obscure, and they are difficult to update if you do not have the required knowledge (actually, only you have such a power!). For me, as a rule of thumb, if you can avoid macros (decl or proc), then avoid them.

I think my main problem is that test_with_all_runtimes assumes that the setup and so on is the same for all runtimes.

Regarding this. The setup is the same only for those types you explicitly instantiate in pallet_xxx::Config. I mean, if I put in the Config trait a bound with pallet_balances::Config<Balance = u128>, then I assume all runtimes use u128, but if I leave it without the restriction, so just pallet_balances::Config, then each runtime could have their own Balance types. So, the idea is only restricted when you know all of our runtimes use the same types, if not, the types are not specified.

For things like AdminOrigin, where maybe the call in the test changes because of that, you can manually dispatch it with an if/match using T::KIND, for such border cases.

@lemunozm
Copy link
Contributor Author

lemunozm commented Oct 2, 2023

But I think, the big point of this PR (and challenge), is that the fudge::companion structs and associated methods and macros, can be abstracted by an Environtment trait. Also if they're abstracted, I think we no longer need macros as env::run or similar, they could be implemented in actual rust code.

@lemunozm
Copy link
Contributor Author

Closed in favor of: #1588

@lemunozm lemunozm closed this Oct 11, 2023
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.

2 participants