Skip to content

fix shared-core-idle-parachain test#8922

Merged
alindima merged 4 commits intomasterfrom
alindima/fix-shared-core-idle-parachain
Jun 20, 2025
Merged

fix shared-core-idle-parachain test#8922
alindima merged 4 commits intomasterfrom
alindima/fix-shared-core-idle-parachain

Conversation

@alindima
Copy link
Copy Markdown
Contributor

Rewrite it with zombienet-sdk, while simplyfing it.
The main source of flakyness was timing, because this test was manually registering parachains, so we had to wait for 2 sessions. Waiting for session change with zndsl zombienet was a hassle, whereas with zombienet-sdk is much easier.

Moreover, I needed to replicate the logic change from: #6554 to the genesis parachain registration, so that we don't automatically get an extra assigned core when registering a parachain (unless we want one)

@alindima alindima requested review from a team as code owners June 20, 2025 11:35
@alindima alindima added the T18-zombienet_tests Trigger zombienet CI tests. label Jun 20, 2025
@alindima alindima requested a review from pepoviola June 20, 2025 11:36
@alindima
Copy link
Copy Markdown
Contributor Author

Ran it 25 times locally and passed each time

Pallet::<T>::initialize_para_now(&mut parachains, *id, genesis_args);
T::AssignCoretime::assign_coretime(*id)
.expect("Assigning coretime works at genesis; qed");
if genesis_args.para_kind == ParaKind::Parachain {
Copy link
Copy Markdown
Contributor

@tdimitrov tdimitrov Jun 20, 2025

Choose a reason for hiding this comment

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

I don't get this. What does with_parachain from zombienet-sdk do so that we enter in this if?
(I looked at the linked PR and still don't get it).

Copy link
Copy Markdown
Contributor Author

@alindima alindima Jun 20, 2025

Choose a reason for hiding this comment

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

By default zombienet-sdk adds parachains to the genesis state (which end up calling this code that I modified). Here is what zombienet ends up doing: https://github.com/paritytech/zombienet-sdk/blob/722993c5665efa0dd74e190729fba95abc2c0b71/crates/orchestrator/src/generators/chain_spec.rs#L670.
Whether we add the para to genesis state is determined by the RegistrationStrategy.

If we set onboard_as_parachain(false) (which I do here), the para_kind will end up being Parathread. And I don't want to have any core assigned if it's a parathread.

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.

Hi @alindima, could be use the RegistrationStrategy::Manual here? With this variant zombienet doesn't register the parachain, only generate the spec/artifacts and spawn the collator. Then you can register the para and assign the core from the test itself.

wdyt?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's what this test used to do, but it's more tedious and it takes one extra session before the parachain starts producing blocks

@alindima alindima added this pull request to the merge queue Jun 20, 2025
Merged via the queue into master with commit e5cf8af Jun 20, 2025
325 of 382 checks passed
@alindima alindima deleted the alindima/fix-shared-core-idle-parachain branch June 20, 2025 15:08
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
Rewrite it with zombienet-sdk, while simplyfing it.
The main source of flakyness was timing, because this test was manually
registering parachains, so we had to wait for 2 sessions. Waiting for
session change with zndsl zombienet was a hassle, whereas with
zombienet-sdk is much easier.

Moreover, I needed to replicate the logic change from:
#6554 to the genesis
parachain registration, so that we don't automatically get an extra
assigned core when registering a parachain (unless we want one)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T18-zombienet_tests Trigger zombienet CI tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants