Skip to content

Conversation

@qiweiii
Copy link
Contributor

@qiweiii qiweiii commented Aug 18, 2025

  • change back to compact encoding for B.10
  • refactor and add back block validations for trace and fuzz tests

@qiweiii qiweiii requested a review from xlc August 18, 2025 08:58
@qiweiii qiweiii enabled auto-merge (squash) August 18, 2025 09:03
@codecov
Copy link

codecov bot commented Aug 18, 2025

Codecov Report

❌ Patch coverage is 87.42515% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.98%. Comparing base (b57478e) to head (904081e).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
Fuzzing/Sources/Fuzzing/FuzzingTarget.swift 0.00% 14 Missing ⚠️
.../Sources/Blockchain/RuntimeProtocols/Runtime.swift 91.66% 2 Missing ⚠️
JAMTests/Tests/JAMTests/jamtestnet/FuzzTests.swift 93.33% 2 Missing ⚠️
.../Sources/Blockchain/RuntimeProtocols/Safrole.swift 93.33% 1 Missing ⚠️
Fuzzing/Sources/Fuzzing/FuzzingClient.swift 0.00% 1 Missing ⚠️
JAMTests/Tests/JAMTests/jamtestnet/TraceTest.swift 85.71% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #351   +/-   ##
=======================================
  Coverage   81.97%   81.98%           
=======================================
  Files         377      377           
  Lines       33022    33067   +45     
=======================================
+ Hits        27071    27111   +40     
- Misses       5951     5956    +5     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

The changes primarily consist of a beneficial refactoring that separates validation logic from state update logic across multiple runtime protocols. This improves code structure and testability. Additionally, the fuzz testing framework has been enhanced to better manage test cases, including those with expected failures. A few minor issues were found: a test was disabled by commenting out its body, a fixed-duration sleep was added to a test, which could indicate a potential race condition, and a debug log was added.


Suggestions that couldn't be attached to a specific line

Node/Tests/NodeTests/NodeTests.swift:117, 138

A fixed-duration Task.sleep has been added to fix a likely race condition. This can make tests slower and potentially flaky. A more robust solution would be to use a deterministic synchronization mechanism, like waiting on an expectation or notification, to confirm that block production is complete before proceeding.


let codeHash: Data32? = try? Data32(state.readMemory(address: regs[0], length: 32))

logger.debug("new codeHash: \(codeHash?.description ?? "nil")")

Choose a reason for hiding this comment

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

A debug log has been added. Please remove this temporary logging before merging.

Comment on lines +11 to +13
// await withKnownIssue("Invalid bandersnatch", isIntermittent: true) {
// try await TraceTest.test(input)
// }

Choose a reason for hiding this comment

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

The body of stfTests is commented out, effectively disabling it. To make the intent clear and ensure the test isn't forgotten, please use the testing framework's official mechanism for disabling tests, such as @Test(.disabled(reason: "Temporarily disabled due to invalid bandersnatch issue")).

@qiweiii qiweiii merged commit af446d5 into master Aug 18, 2025
5 checks passed
@qiweiii qiweiii deleted the fix-fuzz-0816 branch August 18, 2025 23:55
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.

3 participants