feat(contract-e2e): auto-source substrate-contracts-node with e2e tests #254
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peterwht
commented
Jul 18, 2024
peterwht
commented
Jul 18, 2024
peterwht
commented
Jul 18, 2024
peterwht
commented
Jul 18, 2024
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #254 +/- ##
==========================================
- Coverage 70.81% 70.63% -0.18%
==========================================
Files 47 50 +3
Lines 7664 7881 +217
Branches 7664 7881 +217
==========================================
+ Hits 5427 5567 +140
- Misses 1315 1374 +59
- Partials 922 940 +18
|
…#255) * refactor(up-contract): use Binary struct from parachains -> up -> sourcing to auto-launch contracts-node * refactor(contracts_node): reduce duplicated code -- checkpoint, not working * refactor(contracts_node): use Binary and Source structs for substrate-contracts-node * chore: small changes
2 tasks
evilrobot-01
requested changes
Jul 20, 2024
|
Introduced a new |
AlexD10S
reviewed
Jul 22, 2024
|
Looking good, will review again with this merged: #258 before approve |
* refactor(sourcing): move sourcing to pop-common * refactor(sourcing-tests): move sourcing tests to pop-common * refactor(sourcing): better imports * refactor(sourcing): clean sourcing module with better component categorization. Other cleanups
Something wrong after the merge, that tests are failing |
AlexD10S
approved these changes
Jul 24, 2024
There was a problem hiding this comment.
Tests are passing now.
Some decisions need to be made before merging:
- Can be addressed in a separate PR, but we need to decide on this comment: feat(contract-e2e): auto-source substrate-contracts-node with e2e tests #254 (comment),
- Do you want to merge this PR here feat: consistency with
pop up parachainsto handle versioning forcontracts-node#262 to have all the functionality together? or separate in another PR to main?.
|
evilrobot-01
requested changes
Jul 25, 2024
There was a problem hiding this comment.
A minor fix required to the UX and the use of the cache directory for tests need to be improved, as it may currently leave artifacts on users machine if test fails.
See also #263 for additional improvements.
* fix: use async sleep in async context * refactor: remove unnecessary module * refactor: standardise sourcing ux
evilrobot-01
approved these changes
Jul 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Solves #247
pop test contract --features e2e-testsis now deprecated in favor ofpop test contract --e2e.pop test contract --e2ewill now download (if needed)substrate-contracts-nodefor usage in e2e tests.substrate-contracts-nodestandalone command exists, this will be used.pop up contractwas refactored to use the existing functionality for sourcing binaries inpop-parachains.Merge Prerequisite: #258
Review this PR first before #258. #258 moves the sourcing functionality to
pop-common.