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

fix(claims): update Alice's signature in claims bench test #142

Merged
merged 8 commits into from
Mar 14, 2021

Conversation

green-jay
Copy link
Contributor

No description provided.

@enthusiastmartin
Copy link
Contributor

Still getting the same error unfortunately.

---- benchmarking::tests::test_benchmarks stdout ----
thread 'benchmarking::tests::test_benchmarks' panicked at 'Expected Ok(_). Got Err(
    "NoClaimOrAlreadyClaimed",
)', pallets/claims/src/benchmarking.rs:35:13

@jak-pan jak-pan changed the title fix: 🐛 update Alice's signature in claims bench test fix: Alice's signature in claims bench test Mar 11, 2021
@green-jay
Copy link
Contributor Author

green-jay commented Mar 11, 2021

@jak-pan why the commit message rename? AFAIK conventional commits should be in imperative form... although thinking about it, it is indeed more natural like "fix: noun". Btw. where can I find the guidelines?

@green-jay
Copy link
Contributor Author

@enthusiastmartin this is strange, I get test result OK. What command do you use?

@enthusiastmartin
Copy link
Contributor

@enthusiastmartin this is strange, I get test result OK. What command do you use?

to run benchmark tests - you have to buld with runtime-benchmarks features.

cargo test --features runtime-benchmarks -p pallet-claims

@green-jay
Copy link
Contributor Author

@enthusiastmartin hmm, that way I get

error: --features is not allowed in the root of a virtual workspace
note: while this was previously accepted, it didn't actually do anything

@jak-pan
Copy link
Contributor

jak-pan commented Mar 11, 2021

@jak-pan why the commit message rename? AFAIK conventional commits should be in imperative form... although thinking about it, it is indeed more natural like "fix: noun". Btw. where can I find the guidelines?

Imperative is just a good practice but you are right we should use it if possible, guidelines are being worked on. The update was more of a 🐛 removal than about update word so we can readd that.

@enthusiastmartin
Copy link
Contributor

@enthusiastmartin hmm, that way I get

error: --features is not allowed in the root of a virtual workspace
note: while this was previously accepted, it didn't actually do anything

run it inside node directory.

@jak-pan jak-pan changed the title fix: Alice's signature in claims bench test fix: update Alice's signature in claims bench test Mar 11, 2021
@green-jay
Copy link
Contributor Author

thanks, now I get the error too, will check it

Copy link
Contributor

@enthusiastmartin enthusiastmartin left a comment

Choose a reason for hiding this comment

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

One last minor issue. but then lgtm.

pallets/claims/src/benchmarking.rs Outdated Show resolved Hide resolved
@enthusiastmartin enthusiastmartin linked an issue Mar 13, 2021 that may be closed by this pull request
@jak-pan jak-pan changed the title fix: update Alice's signature in claims bench test fix(claims): update Alice's signature in claims bench test Mar 13, 2021
@jak-pan
Copy link
Contributor

jak-pan commented Mar 13, 2021

bump cargo.toml and implementation version pls :)

@jak-pan jak-pan self-requested a review March 13, 2021 10:56
runtime/Cargo.toml Outdated Show resolved Hide resolved
@enthusiastmartin enthusiastmartin merged commit a925b0a into master Mar 14, 2021
@jak-pan jak-pan deleted the fix/claims_bench_verify branch March 14, 2021 16:55
@jak-pan jak-pan changed the title fix(claims): update Alice's signature in claims bench test fix(claims): update Alice's signature in claims bench test Mar 21, 2021
@auto-add-label auto-add-label bot added the bug Something isn't working label Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Claim benchmarking test
3 participants