Use parameter_types instead of thread_local for test-setup #12036

tifecool · 2022-08-15T22:20:39Z

PR for: #12020

WIP

Fixes: #12020
might close: #10479

polkadot address: 1s51dYXziQwTnSnModErKfra3TeEsVipqzY846CLCTEXtHJ

paritytech-cicd-pr · 2022-08-15T22:22:15Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1746777

paritytech-cicd-pr · 2022-08-15T22:22:18Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1746778

paritytech-cicd-pr · 2022-08-15T23:00:36Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1746999

paritytech-cicd-pr · 2022-08-15T23:00:39Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1746998

frame/assets/src/mock.rs

kianenigma

Good direction, but:

naming needs fixing
Please add https://github.com/paritytech/substrate/pull/12002/files#diff-31d6476b566f793c17f32f9fdefeb487988af58c61ba752d0275b91f6c8aaccaR433 to your PR and use it instead of :get and :set when appropriate

Co-authored-by: Kian Paimani <[email protected]>

paritytech-cicd-pr · 2022-08-16T08:51:46Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1748220

paritytech-cicd-pr · 2022-08-16T09:15:08Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1748335

paritytech-cicd-pr · 2022-08-16T09:23:59Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1748405

paritytech-cicd-pr · 2022-08-16T16:06:15Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750233

paritytech-cicd-pr · 2022-08-16T17:44:05Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1750594

frame/aura/src/mock.rs

paritytech-cicd-pr · 2022-08-17T22:02:24Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1754151

paritytech-cicd-pr · 2022-08-17T22:02:24Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1754150

paritytech-cicd-pr · 2022-08-17T22:02:26Z

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1754152

kianenigma · 2022-08-27T05:12:29Z

frame/im-online/src/tests.rs

@@ -96,7 +97,8 @@ fn should_report_offline_validators() {
 		advance_session();

 		// then
-		let offences = OFFENCES.with(|l| l.replace(vec![]));
+		let offences = OFFENCES::get();


Not applied yet?

kianenigma · 2022-08-27T05:12:50Z

frame/im-online/src/tests.rs

+		let offences = OFFENCES::get();
+		OFFENCES::mutate(|l| l.clear());


Suggested change

let offences = OFFENCES::get();

OFFENCES::mutate(|l| l.clear());

let offences = Offences::take();

kianenigma · 2022-08-27T05:13:15Z

frame/offences/src/mock.rs

-	pub static ON_OFFENCE_PERBILL: RefCell<Vec<Perbill>> = RefCell::new(Default::default());
-	pub static OFFENCE_WEIGHT: RefCell<Weight> = RefCell::new(Default::default());
+parameter_types! {
+	pub static OnOffencePerBill: Vec<Perbill> = Default::default();


Suggested change

pub static OnOffencePerBill: Vec<Perbill> = Default::default();

pub static OnOffencePerbill: Vec<Perbill> = Default::default();

kianenigma · 2022-08-27T05:13:22Z

frame/offences/src/mock.rs

@@ -58,16 +57,16 @@ impl<Reporter, Offender> offence::OnOffenceHandler<Reporter, Offender, Weight>
 		_offence_session: SessionIndex,
 		_disable_strategy: DisableStrategy,
 	) -> Weight {
-		ON_OFFENCE_PERBILL.with(|f| {
-			*f.borrow_mut() = slash_fraction.to_vec();
+		OnOffencePerBill::mutate(|f| {


Suggested change

OnOffencePerBill::mutate(|f| {

OnOffencePerbill::mutate(|f| {

kianenigma

LGTM modulo some last nits, please make sure all open comments are addressed.

kianenigma · 2022-08-27T05:19:21Z

/tip large

substrate-tip-bot · 2022-08-27T05:19:24Z

@kianenigma A large tip was successfully submitted for tifecool (1s51dYXziQwTnSnModErKfra3TeEsVipqzY846CLCTEXtHJ on polkadot).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/treasury/tips

tifecool · 2022-08-27T05:22:55Z

Wow. Really appreciate the tip! Will round up immediately ❤️

frame/support/src/lib.rs

bkchr · 2022-09-02T20:07:35Z

@tifecool please merge master

tifecool · 2022-09-05T17:36:27Z

@athei please review

kianenigma · 2022-09-06T14:13:04Z

The ::take seem to have caused some issues in Polkadot.

Let's do @bkchr's suggestion in #12036 (comment) and use $value, not Default.

Please document this in the ::take() function! it is a bit of strange behavior.

tifecool · 2022-09-07T09:51:32Z

Let's do @bkchr's suggestion in #12036 (comment) and use $value, not Default.

Ok, let me try.

athei · 2022-09-07T11:13:03Z

frame/contracts/src/exec.rs

-	#[derive(Default)]
-	struct MockLoader {
+	#[derive(Default, Clone)]
+	pub struct MockLoader {


Why does it need to be public?

parameter_types requires it

athei · 2022-09-07T11:13:29Z

frame/contracts/src/storage/meter.rs

-	#[derive(Default, Debug, PartialEq, Eq)]
-	struct TestExt {
+	#[derive(Default, Debug, PartialEq, Eq, Clone)]
+	pub struct TestExt {


Why does it need to be public?

parameter_types requires it

kianenigma · 2022-09-08T10:17:26Z

Are we sure this closes #10479 too?

frame/support/src/lib.rs

kianenigma · 2022-09-08T10:19:17Z

bot merge

paritytech-processbot · 2022-09-08T10:19:25Z

Waiting for commit status.

athei · 2022-09-08T10:21:26Z

Are we sure this closes #10479 too?

It still uses thread local under the hood. So I don't think so.

…h#12036) * Edit to Assets. parameter_types * fixes * Test Fixes. WIP * Edits to pallet-aura * Camel Case Co-authored-by: Kian Paimani <[email protected]> * Implementation of mutate fn * update to pallet-aura * Update to frame-system. Fixes * Update to frame-support-test. CamelCases * Updates to frame- contracts, offences, staking, bounties, child bounties * Edit to mutate fn. Changes to frame-contracts. CamelCase pallet-aura * Edits to frame-contracts & executive * cargo +nightly fmt * unused import removed * unused import removed * cargo +nightly fmt * minor adjustment * updates * updates * cargo +nightly fmt * cargo +nightly fmt * take fn implemented * update * update * Fixes to CallFilter * cargo +nightly fmt * final fixes * Default changed to $value * Update frame/support/src/lib.rs Co-authored-by: Kian Paimani <[email protected]>

tifecool added 3 commits August 15, 2022 11:40

Edit to Assets. parameter_types

aed1d68

fixes

04ce047

Test Fixes. WIP

1062686

tifecool added 3 commits August 15, 2022 23:23

Merge branch 'master' into fix12020

de78b6b

Edits to pallet-aura

42bbc26

Merge remote-tracking branch 'origin/fix12020' into fix12020

7959071

tifecool changed the title ~~Edit to Assets. parameter_types~~ Use parameter_types instead of thread_local for test-setup Aug 15, 2022

kianenigma reviewed Aug 16, 2022

View reviewed changes

frame/assets/src/mock.rs Outdated Show resolved Hide resolved

kianenigma suggested changes Aug 16, 2022

View reviewed changes

Camel Case

1694082

Co-authored-by: Kian Paimani <[email protected]>

tifecool added 2 commits August 16, 2022 10:13

Implementation of mutate fn

d6ff5db

Merge remote-tracking branch 'origin/fix12020' into fix12020

29a4210

update to pallet-aura

83b6304

Update to frame-system. Fixes

2e3ff7c

Update to frame-support-test. CamelCases

fa0adc6

tifecool commented Aug 16, 2022

View reviewed changes

frame/aura/src/mock.rs Outdated Show resolved Hide resolved

Updates to frame- contracts, offences, staking, bounties, child bounties

f175b7f

tifecool requested a review from athei as a code owner August 17, 2022 21:59

kianenigma reviewed Aug 27, 2022

View reviewed changes

kianenigma approved these changes Aug 27, 2022

View reviewed changes

final fixes

48f95b6

bkchr approved these changes Sep 2, 2022

View reviewed changes

frame/support/src/lib.rs Outdated Show resolved Hide resolved

Merge branch 'master' into fix12020

91be0b4

Default changed to $value

9efdf6e

athei reviewed Sep 7, 2022

View reviewed changes

kianenigma reviewed Sep 8, 2022

View reviewed changes

frame/support/src/lib.rs Outdated Show resolved Hide resolved

Update frame/support/src/lib.rs

7beef9f

kianenigma added the A8-mergeoncegreen label Sep 8, 2022

paritytech-processbot bot merged commit 0478535 into paritytech:master Sep 8, 2022

tifecool deleted the fix12020 branch September 8, 2022 15:56

This was referenced Oct 21, 2022

Member Request tifecool/seeding#1

Closed

Member Request polkadot-fellows/seeding#46

Closed

zjb0807 mentioned this pull request Feb 27, 2024

Use parameter_types instead of thread_local AcalaNetwork/Acala#2719

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use parameter_types instead of thread_local for test-setup #12036

Use parameter_types instead of thread_local for test-setup #12036

tifecool commented Aug 15, 2022 •

edited by kianenigma

Loading

paritytech-cicd-pr commented Aug 15, 2022

paritytech-cicd-pr commented Aug 15, 2022

paritytech-cicd-pr commented Aug 15, 2022

paritytech-cicd-pr commented Aug 15, 2022

kianenigma left a comment •

edited

Loading

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 17, 2022

paritytech-cicd-pr commented Aug 17, 2022

paritytech-cicd-pr commented Aug 17, 2022

kianenigma Aug 27, 2022

tifecool Aug 27, 2022

kianenigma Aug 27, 2022

kianenigma Aug 27, 2022

kianenigma Aug 27, 2022

kianenigma left a comment

kianenigma commented Aug 27, 2022

substrate-tip-bot bot commented Aug 27, 2022

tifecool commented Aug 27, 2022

bkchr commented Sep 2, 2022

tifecool commented Sep 5, 2022

kianenigma commented Sep 6, 2022 •

edited

Loading

tifecool commented Sep 7, 2022

athei Sep 7, 2022

tifecool Sep 8, 2022

athei Sep 7, 2022

tifecool Sep 8, 2022

kianenigma commented Sep 8, 2022

kianenigma commented Sep 8, 2022

paritytech-processbot bot commented Sep 8, 2022

athei commented Sep 8, 2022

		let offences = OFFENCES::get();
		OFFENCES::mutate(\|l\| l.clear());

	let offences = OFFENCES::get();
	OFFENCES::mutate(\|l\| l.clear());
	let offences = Offences::take();

	pub static OnOffencePerBill: Vec<Perbill> = Default::default();
	pub static OnOffencePerbill: Vec<Perbill> = Default::default();

	OnOffencePerBill::mutate(\|f\| {
	OnOffencePerbill::mutate(\|f\| {

Use parameter_types instead of thread_local for test-setup #12036

Use parameter_types instead of thread_local for test-setup #12036

Conversation

tifecool commented Aug 15, 2022 • edited by kianenigma Loading

paritytech-cicd-pr commented Aug 15, 2022

paritytech-cicd-pr commented Aug 15, 2022

paritytech-cicd-pr commented Aug 15, 2022

paritytech-cicd-pr commented Aug 15, 2022

kianenigma left a comment • edited Loading

Choose a reason for hiding this comment

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 16, 2022

paritytech-cicd-pr commented Aug 17, 2022

paritytech-cicd-pr commented Aug 17, 2022

paritytech-cicd-pr commented Aug 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kianenigma left a comment

Choose a reason for hiding this comment

kianenigma commented Aug 27, 2022

substrate-tip-bot bot commented Aug 27, 2022

tifecool commented Aug 27, 2022

bkchr commented Sep 2, 2022

tifecool commented Sep 5, 2022

kianenigma commented Sep 6, 2022 • edited Loading

tifecool commented Sep 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kianenigma commented Sep 8, 2022

kianenigma commented Sep 8, 2022

paritytech-processbot bot commented Sep 8, 2022

athei commented Sep 8, 2022

tifecool commented Aug 15, 2022 •

edited by kianenigma

Loading

kianenigma left a comment •

edited

Loading

kianenigma commented Sep 6, 2022 •

edited

Loading