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

Improve runtime tests #1001

Merged
merged 11 commits into from
Nov 13, 2023
Merged

Improve runtime tests #1001

merged 11 commits into from
Nov 13, 2023

Conversation

yrong
Copy link
Contributor

@yrong yrong commented Nov 9, 2023

No description provided.

Copy link
Collaborator

@vgeddes vgeddes left a comment

Choose a reason for hiding this comment

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

Nice! 🚀

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8e921b2) 81.14% compared to head (633dd29) 81.23%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1001      +/-   ##
==========================================
+ Coverage   81.14%   81.23%   +0.09%     
==========================================
  Files          53       53              
  Lines        2132     2132              
  Branches       72       72              
==========================================
+ Hits         1730     1732       +2     
+ Misses        387      385       -2     
  Partials       15       15              
Flag Coverage Δ
rust 81.31% <ø> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yrong yrong marked this pull request as ready for review November 9, 2023 13:07
Copy link
Collaborator

@vgeddes vgeddes left a comment

Choose a reason for hiding this comment

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

Excellent!

@yrong
Copy link
Contributor Author

yrong commented Nov 10, 2023

@claravanstaden I've moved the tests in Snowfork/polkadot-sdk#17 to our own repo and made some fix. To seperate from cumulus it's more convenient to maintain/extend tests as required IMO.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make this small change. Move this crate to directory parachain/runtime/tests/Cargo.toml.

Since @alistair-singh is also introducing a runtime-common crate, we should put all the runtime specific crates under parachain/runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--manifest-path polkadot-sdk/Cargo.toml
-p bridge-hub-rococo-integration-tests
-p bridge-hub-rococo-integration-tests snowbridge
Copy link
Collaborator

Choose a reason for hiding this comment

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

What package is named snowbridge? I don't recall any package like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually it's not a package name, the mod name to filter snowbrige tests only.

@yrong yrong merged commit e9919c5 into main Nov 13, 2023
7 checks passed
@yrong yrong deleted the ron/runtime-tests branch November 13, 2023 00:12
@@ -12,6 +12,6 @@ members = [
"pallets/ethereum-beacon-client",
"pallets/control",
"pallets/control/runtime-api",
"runtime/tests",
Copy link
Collaborator

@vgeddes vgeddes Nov 13, 2023

Choose a reason for hiding this comment

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

The tests crate is only used by the runtimes in polkadot-sdk/, so I don't think we should add it to the cargo workspace. We're effectively creating some kind of cyclical dependency loop here :)

Also because its going to bloat our local compile times.

When I run cargo test, I just want to run tests for our pallets and primitives crates. Not runtime-specific stuff.

So its fine for the crate to have its own Cargo.lock.

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