Skip to content

Conversation

@jferrant
Copy link
Contributor

@jferrant jferrant commented Sep 23, 2025

Need to remove duplication in TestPeer by having it hold a TestChainstate under the hood but wanted to open this sep as my attempt to do so had introduced some bug somewhere.

Closes #6523

Signed-off-by: Jacinta Ferrant <jacinta.ferrant@gmail.com>
@jferrant jferrant requested a review from a team as a code owner September 23, 2025 19:14
@jferrant jferrant added the aac Avoiding Accidental Consensus label Sep 23, 2025
@jferrant jferrant requested a review from a team as a code owner September 23, 2025 19:14
@jferrant jferrant added the aac-testing Avoiding Accidental Consensus Testing Specific Task label Sep 23, 2025
Signed-off-by: Jacinta Ferrant <jacinta.ferrant@gmail.com>
@jferrant jferrant force-pushed the feat/aac-testing-boiler-plate branch from a883183 to 2376c98 Compare September 23, 2025 19:46
Copy link
Contributor

@fdefelici fdefelici left a comment

Choose a reason for hiding this comment

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

Great start! this looks really promising!

I’ve left a few comments for discussion.

Signed-off-by: Jacinta Ferrant <jacinta.ferrant@gmail.com>
…me struct

Signed-off-by: Jacinta Ferrant <jacinta.ferrant@gmail.com>
@jferrant jferrant requested review from Jiloc and fdefelici September 26, 2025 20:41
…N string

Signed-off-by: Jacinta Ferrant <jacinta.ferrant@gmail.com>
Signed-off-by: Jacinta Ferrant <jacinta.ferrant@gmail.com>
Signed-off-by: Jacinta Ferrant <jacinta.ferrant@gmail.com>
@jferrant jferrant force-pushed the feat/aac-testing-boiler-plate branch from e566692 to b6fc3ff Compare September 29, 2025 23:03
@jferrant jferrant requested a review from fdefelici September 30, 2025 17:37
Jiloc
Jiloc previously approved these changes Sep 30, 2025
Copy link
Contributor

@Jiloc Jiloc left a comment

Choose a reason for hiding this comment

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

Good job with setup! I believe there are aspects we can refine further, but they will become clearer when we'll start creating more tests. For now they could be too premature. I am fine with going forwad with this as-is. We can refactor the specific aspects in future PRs

Signed-off-by: Jacinta Ferrant <jacinta.ferrant@gmail.com>
Signed-off-by: Jacinta Ferrant <jacinta.ferrant@gmail.com>
@fdefelici
Copy link
Contributor

Good job with setup! I believe there are aspects we can refine further, but they will become clearer when we'll start creating more tests. For now they could be too premature. I am fine with going forwad with this as-is. We can refactor the specific aspects in future PRs

If your preference is to merge this PR as-is, I'm fine with that.

Just to recap a few open points we have that could be addressed in follow-up PRs:

  • marf computation discussion: This is still in progress. I’m experimenting with different ways to compute the MARF, but haven’t achieved the expected result yet.
  • Expected Failure matching discussion: There are a couple of proposals on the table. I could try implementing the approach I proposed, in a separate PR.

@Jiloc
Copy link
Contributor

Jiloc commented Oct 1, 2025

Good job with setup! I believe there are aspects we can refine further, but they will become clearer when we'll start creating more tests. For now they could be too premature. I am fine with going forwad with this as-is. We can refactor the specific aspects in future PRs

If your preference is to merge this PR as-is, I'm fine with that.

Just to recap a few open points we have that could be addressed in follow-up PRs:

  • marf computation discussion: This is still in progress. I’m experimenting with different ways to compute the MARF, but haven’t achieved the expected result yet.
  • Expected Failure matching discussion: There are a couple of proposals on the table. I could try implementing the approach I proposed, in a separate PR.

note that IF we'll go forward with jferrant#3 , the "Expected Failure matching" point will be automatically handled by it

@fdefelici
Copy link
Contributor

fdefelici commented Oct 2, 2025

note that IF we'll go forward with jferrant#3 , the "Expected Failure matching" point will be automatically handled by it

Well, it would be more correct say that if we are fine with the string match approach, it is already managed!

And in this case, if the intention is to (try to) trigger all kind of failure, probably we'll need at least to remap the OS related error that are dynamic due to the localization.

@jferrant jferrant enabled auto-merge October 2, 2025 18:05
@jferrant jferrant added this pull request to the merge queue Oct 2, 2025
@jferrant jferrant removed this pull request from the merge queue due to a manual request Oct 2, 2025
@jferrant jferrant added this pull request to the merge queue Oct 2, 2025
Merged via the queue into stacks-network:develop with commit b5e3baf Oct 2, 2025
1 of 2 checks passed
@jferrant jferrant deleted the feat/aac-testing-boiler-plate branch October 2, 2025 18:57
@codecov
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 88.15959% with 184 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.55%. Comparing base (5934517) to head (857c0f9).
⚠️ Report is 93 commits behind head on develop.

Files with missing lines Patch % Lines
stackslib/src/chainstate/tests/mod.rs 83.40% 120 Missing ⚠️
stackslib/src/chainstate/tests/consensus.rs 91.36% 52 Missing ⚠️
stackslib/src/chainstate/nakamoto/tests/node.rs 89.65% 6 Missing ⚠️
stackslib/src/net/tests/mod.rs 96.49% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6531      +/-   ##
===========================================
+ Coverage    75.36%   80.55%   +5.18%     
===========================================
  Files          565      568       +3     
  Lines       345043   347203    +2160     
===========================================
+ Hits        260037   279679   +19642     
+ Misses       85006    67524   -17482     
Files with missing lines Coverage Δ
stackslib/src/chainstate/nakamoto/tests/node.rs 83.73% <89.65%> (+0.05%) ⬆️
stackslib/src/net/tests/mod.rs 97.02% <96.49%> (-0.08%) ⬇️
stackslib/src/chainstate/tests/consensus.rs 91.36% <91.36%> (ø)
stackslib/src/chainstate/tests/mod.rs 83.40% <83.40%> (ø)

... and 284 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5934517...857c0f9. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

aac Avoiding Accidental Consensus aac-testing Avoiding Accidental Consensus Testing Specific Task locked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AAC Testing: Develop Integration Test Harness for append_block in stackslib

3 participants