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.

Sign up for GitHub

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

Jump to bottom

bench syntax v2: run test in single functions #376

Closed
Tracked by #1428
, #380
...
ggwpez opened this issue Mar 28, 2023 · 6 comments · Fixed by #3574
Closed
Tracked by #1428
, #380
...

bench syntax v2: run test in single functions #376

ggwpez opened this issue Mar 28, 2023 · 6 comments · Fixed by #3574
Assignees
@pandres95
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. T12-benchmarks This PR/Issue is related to benchmarking and weights.

Comments

@ggwpez
Copy link
Member

ggwpez commented Mar 28, 2023

The v2 benchmark tests are running in one single function instead of one function per bench case.

Current output:

test benchmark_tests::test_benchmarks ... ok

Output that we want:

test benchmarking::bench_sort_vector ... ok
test benchmarking::bench_accumulate_dummy ... ok
test benchmarking::bench_set_dummy_benchmark ... ok

It is probably using this wrong expansion arm here:
https://github.com/paritytech/substrate/blob/2e535bac47f48d816f94b105715e09dc57ed52b0/frame/benchmarking/src/v1.rs#L541

instead of:
https://github.com/paritytech/substrate/blob/2e535bac47f48d816f94b105715e09dc57ed52b0/frame/benchmarking/src/v1.rs#L508
@ggwpez ggwpez changed the title v2 bench syntax: run test in single files bench syntax v2: run test in single functions Mar 28, 2023
@ggwpez ggwpez mentioned this issue Mar 29, 2023
Closed
27 tasks
@sam0x17 sam0x17 self-assigned this Apr 10, 2023
@NingLin-P
Copy link

Friendly ping, we are using the bench syntax v2 in https://github.com/subspace/subspace, and this feature is highly desired by our usage.

@ggwpez
Copy link
Member Author

ggwpez commented May 1, 2023 •
edited

Okay thanks for the feedback @NingLin-P, will look into it soon.
Feel free to post additional feedback into #380, since the syntax is not finalized yet.

@NingLin-P NingLin-P mentioned this issue May 1, 2023
Closed
@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added T10-tests This PR/Issue is related to tests. and removed I5-tests labels Aug 25, 2023
@JuaniRios
Copy link
Contributor

Would like to have this as well in our codebase at Polimec

@sam0x17
Copy link
Contributor

sam0x17 commented Sep 14, 2023

This will most likely be a top priority for me once #1343 is done

@ggwpez ggwpez added C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. T12-benchmarks This PR/Issue is related to benchmarking and weights. and removed T10-tests This PR/Issue is related to tests. labels Feb 4, 2024
@pandres95 pandres95 mentioned this issue Mar 5, 2024
Merged
@pandres95
Copy link
Contributor

@ggwpez I already started solving this issue.

@pandres95
Copy link
Contributor

@ggwpez PR's ready for review.

github-merge-queue bot pushed a commit that referenced this issue Mar 8, 2024
@pandres95
fix(pallet-benchmarking): split test functions in v2 (#3574)
f4fbdde 
Closes #376

---------

Co-authored-by: command-bot <>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this issue Mar 25, 2024
@pandres95
fix(pallet-benchmarking): split test functions in v2 (paritytech#3574)
53dfdef 
Closes paritytech#376

---------

Co-authored-by: command-bot <>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
ed16e13 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
14d2aae 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
27f403f 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
ee425ed 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
3212984 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
1acdc9c 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
ce5b911 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
2841e49 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
1d3666b 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
@svyatonik @serban300
Substrate relay stub (paritytech#376)
e3c0016 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
jonathanudd pushed a commit to jonathanudd/polkadot-sdk that referenced this issue Apr 10, 2024
@svyatonik @bkchr
Substrate relay stub (paritytech#376)
dbb72fa 
* substrate-relay: initial commit

* MillauHeaderApi and RialtoHeaderApi

* post-merge fixes + TODOs + compilation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pandres95 pandres95

Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
Benchmarking and Weights
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(pallet-benchmarking): split test functions in v2 pandres95/polkadot-sdk
6 participants
@sam0x17 @pandres95 @ggwpez @NingLin-P @JuaniRios @the-right-joyce