Skip to content

feat(spec/test): Model-based tests for part streaming #814

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

Merged
merged 22 commits into from
Feb 11, 2025

Conversation

stojanovic00
Copy link
Contributor

  • Document proposal part streaming quint specification

  • Document proposal part streaming implementation

  • Remove redundant state variables (=> update invariants)

  • Add concrete tests in quint spec

    • Fix symbolinc link from mbt/test to specs
  • Generate and parse itf-trace

Closes: #763

* Document proposal part streaming quint specification

* Document proposal part streaming implementation

* Remove redundant state variables (=> update invariants)

* Add concrete tests in quint spec
  * Fix symbolinc link from mbt/test to specs

* Generate and parse itf-trace
@hvanz hvanz added work in progress Work in progress mbt Model-Based Testing labels Jan 29, 2025
@romac romac added the spec Related to specifications label Jan 30, 2025
* supplemented missing proto files to test-mempool

* added Buffer parsing

* all invariant checks
Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.12%. Comparing base (fec4b6e) to head (73a4400).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #814      +/-   ##
==========================================
+ Coverage   75.87%   76.12%   +0.24%     
==========================================
  Files         172      175       +3     
  Lines       14785    14905     +120     
==========================================
+ Hits        11218    11345     +127     
+ Misses       3567     3560       -7     
Flag Coverage Δ
integration 75.87% <ø> (+0.28%) ⬆️
mbt 8.52% <ø> (-6.14%) ⬇️

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

Components Coverage Δ
core ∅ <ø> (∅)
engine ∅ <ø> (∅)
app ∅ <ø> (∅)
starknet ∅ <ø> (∅)

@stojanovic00
Copy link
Contributor Author

  • Update and expand the documentation of the code and the spec

  • Extend the spec with new tests and invariants

    • Fix runtime bugs and remove redundant invariants
  • Write in Rust the types and data structures that will be used to map the spec's state variables

  • Write MBT test, which generates ITF traces from the Quint spec and runs the traces as Rust tests

    • Update/fix symbolic link from test/mbt to specs
  • Write implementation of ItfRunner for ITF traces generated from the spec

    • Fix dependency issues with used crates
  • Integrate the new MBT tests in CI

Closes: #763

@stojanovic00 stojanovic00 marked this pull request as ready for review February 3, 2025 13:47
@hvanz hvanz self-requested a review February 3, 2025 14:03
Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

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

Left some minor comments, looks great otherwise! 🚀

Will do a deeper pass later today or tomorrow.

@romac romac added code Code/implementation related and removed work in progress Work in progress labels Feb 3, 2025
Copy link
Member

@hvanz hvanz left a comment

Choose a reason for hiding this comment

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

Great work @stojanovic00! 🚀

@hvanz
Copy link
Member

hvanz commented Feb 4, 2025

@romac Now I realize that the Starknet related specs were moved to its own directory. Should these new MBT tests be also moved to its own directory, so it will be easier to move then later to a separate repo?

* adde spec to impl type converters

* extract dummy data generators to utils

* delete unnecessary comments

* rename test
@romac romac changed the title feat(spec/test): part streaming mbt - itf-trace parsing feat(spec/test): Model-based tests for part streaming Feb 4, 2025
@romac
Copy link
Member

romac commented Feb 4, 2025

Yes, let's move that spec under the appropriate folder under specs/

Copy link
Member

@hvanz hvanz left a comment

Choose a reason for hiding this comment

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

I see that now all CI tests are passing and the MBT tests was moved under the starknet crate. Thanks!

Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

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

Amazing work! Pre-approving with a few nitpicks. Looks great otherwise!

@stojanovic00
Copy link
Contributor Author

I appreciate the thorough reviews @hvanz and @romac ! I hope I have addressed all the comments.

@hvanz hvanz added this pull request to the merge queue Feb 11, 2025
Merged via the queue into informalsystems:main with commit f1d53f5 Feb 11, 2025
22 checks passed
@stojanovic00 stojanovic00 deleted the part-streaming-mbt branch February 11, 2025 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Code/implementation related mbt Model-Based Testing spec Related to specifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: MBT for Part Streaming
3 participants