-
Notifications
You must be signed in to change notification settings - Fork 679
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
[pox-4] Pool Stacking stateful property-based tests #4550
Conversation
contrib/core-contract-tests/tests/pox-4/pox-4.stateful-prop.test.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #4550 +/- ##
==========================================
- Coverage 83.36% 82.97% -0.40%
==========================================
Files 470 470
Lines 332151 332768 +617
Branches 317 317
==========================================
- Hits 276905 276100 -805
- Misses 55238 56660 +1422
Partials 8 8 see 77 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
b5814a4
to
b86104d
Compare
contrib/core-contract-tests/tests/pox-4/pox-4.stateful-prop.test.ts
Outdated
Show resolved
Hide resolved
contrib/core-contract-tests/tests/pox-4/pox_DelegateStxCommand.ts
Outdated
Show resolved
Hide resolved
contrib/core-contract-tests/tests/pox-4/pox_GetStxAccountCommand.ts
Outdated
Show resolved
Hide resolved
contrib/core-contract-tests/tests/pox-4/pox-4.stateful-prop.test.ts
Outdated
Show resolved
Hide resolved
59c0a26
to
9f5ac7c
Compare
contrib/core-contract-tests/tests/pox-4/pox-4.stateful-prop.test.ts
Outdated
Show resolved
Hide resolved
contrib/core-contract-tests/tests/pox-4/pox_RevokeDelegateStxCommand.ts
Outdated
Show resolved
Hide resolved
fd29768
to
5d79ffb
Compare
31f0901
to
e21db00
Compare
Co-authored-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>
…let from ex-allowed's list
…_BURNCHAIN_BLOCK_HEIGHT` consts
3c085f8
to
5679afa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be against dev
.
I reviewed the smaller PRs, so only quickly looked at the whole code again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have seen the individual branches along the way, kudos to the progress, excited to get this merged in!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving a "request for change" so that it doesn't get merged while I investigate
it("statefully interacts with PoX-4", async () => { | ||
// SUT stands for "System Under Test". | ||
const sut: Real = { | ||
network: await initSimnet(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be causing the issue identified by @BowTiedRadone
I'm currently investigating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Can we disable the implicit/global
simnet
object for this test?it("statefully interacts with PoX-4", async () => { // No implicit/global `simnet` object for the scope of this test. });
- Or perhaps I can reference the implicit/global
simnet
instead ofawait initSimnet()
.
FWIW, in the long term I want these tests to run outside of Vitest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we disable the implicit/global simnet object for this test?
That would be doable by tweaking the setup file but I don't see a lot of advantage to do so.
Or perhaps I can reference the implicit/global simnet instead of await initSimnet().
That would make sense yes 👍 It wouldn't fix the issue though, it wouldn't have much of an impact but I still recommend it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I can reference the implicit/global simnet
instead of await initSimnet()
.
@moodmosaic @BowTiedRadone |
Yes. Sounds reasonable. 👍 |
@hugocaillard: @BowTiedRadone is working on a branch to add the remaining solo staking scenarios (this PR adds the pool stacking ones). Perhaps we can add #4550 (comment) and #4550 (comment) there and get this merged now (unless you consider those as blockers, but I think they are not). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good @moodmosaic and great job @BowTiedRadone. It's amazing to look at all of these tests running 🎉
And we just started. Stay tuned 🚀 |
Now that @BowTiedRadone opened #4725, we can do that (inside that PR or probably better as a separate one). @hugocaillard, in #4698 the In our case, then, we are going to do the same with |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
This is an integral part of our commitment to adopting a stateful property-based testing strategy for PoX-4.
Applicable issues
Additional info (benefits, drawbacks, caveats)
The stateful property-based tests can run via
npm test
and also directly viaSample output