Skip to content

feat: rework test-harness for integration tests#1039

Merged
bitwalker merged 4 commits into0xMiden:nextfrom
lambdaclass:fabrizioorsi/i876-harness-in-tests
Mar 30, 2026
Merged

feat: rework test-harness for integration tests#1039
bitwalker merged 4 commits into0xMiden:nextfrom
lambdaclass:fabrizioorsi/i876-harness-in-tests

Conversation

@lima-limon-inc
Copy link
Copy Markdown
Contributor

@lima-limon-inc lima-limon-inc commented Mar 27, 2026

Tackles #981

This PR aims to remove some functionality from the compiler's testing crate and rely on miden-protocol's testing crate.
This includes the following changes:

  • Removed the create_note_from_package function. This now relies on miden-protocol's NoteBuilder (which got Package support added into it in this PR).
  • Removed the account_component_from_package function which was replaced by AccountComponent::from_package.
  • Removed build_existing_basic_wallet_account_builder function in favor ofMockChainBuilder::add_existing_account_from_components which was also added in PR
  • Removed NoteCreationConfig since it was superseded by NoteBuilder.

Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
Suggested-by: Dennis Zadorozhnyi <denys.z@miden.team>
Signed-off-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com>
@lima-limon-inc lima-limon-inc force-pushed the fabrizioorsi/i876-harness-in-tests branch from a74cccc to e6df139 Compare March 27, 2026 14:33
@lima-limon-inc
Copy link
Copy Markdown
Contributor Author

@greenhat

Since #896 got closed, I re-opened this PR with your comments addressed. I have re-based it onto the newest changes in next.

Copy link
Copy Markdown
Collaborator

@bitwalker bitwalker left a comment

Choose a reason for hiding this comment

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

So the problem with the removal of compile_rust_package is that by calling miden build, we're not actually testing the right version of the compiler (i.e. we're testing whatever the current midenup toolchain has installed). Using miden build is correct for other parts of the project, but not the compiler itself (where we need to test things based on the current source of the compiler).

I think we probably need to keep compile_rust_package unfortunately, or have it invoke cargo miden build instead (and ensure that it is looking in the right place for the cargo-miden binary - which our test suite currently does, but if responsibility for that moves somewhere else, we'll need a way to ensure the right binaries are being used).

Let me know if I've misunderstood something though!

@greenhat
Copy link
Copy Markdown
Contributor

So the problem with the removal of compile_rust_package is that by calling miden build, we're not actually testing the right version of the compiler (i.e. we're testing whatever the current midenup toolchain has installed). Using miden build is correct for other parts of the project, but not the compiler itself (where we need to test things based on the current source of the compiler).

I think we probably need to keep compile_rust_package unfortunately, or have it invoke cargo miden build instead (and ensure that it is looking in the right place for the cargo-miden binary - which our test suite currently does, but if responsibility for that moves somewhere else, we'll need a way to ensure the right binaries are being used).

Let me know if I've misunderstood something though!

I think this PR description point was not updated. compile_rust_package is unchanged in this PR, meaning that it still calls the cargo-miden library. The miden build version is planned for the user projects and is discussed at 0xMiden/protocol#2611. The compiler test suite will continue using the current compile_rust_package calling cargo-miden library.

Copy link
Copy Markdown
Contributor

@greenhat greenhat left a comment

Choose a reason for hiding this comment

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

Looking good! Nice work!

@greenhat greenhat requested a review from bitwalker March 30, 2026 10:18
@lima-limon-inc
Copy link
Copy Markdown
Contributor Author

So the problem with the removal of compile_rust_package is that by calling miden build, we're not actually testing the right version of the compiler (i.e. we're testing whatever the current midenup toolchain has installed). Using miden build is correct for other parts of the project, but not the compiler itself (where we need to test things based on the current source of the compiler).
I think we probably need to keep compile_rust_package unfortunately, or have it invoke cargo miden build instead (and ensure that it is looking in the right place for the cargo-miden binary - which our test suite currently does, but if responsibility for that moves somewhere else, we'll need a way to ensure the right binaries are being used).
Let me know if I've misunderstood something though!

I think this PR description point was not updated. compile_rust_package is unchanged in this PR, meaning that it still calls the cargo-miden library. The miden build version is planned for the user projects and is discussed at 0xMiden/protocol#2611. The compiler test suite will continue using the current compile_rust_package calling cargo-miden library.

@bitwalker Indeed, my mistake, the PR description contained items from a previous iteration of this PR. I have now updated the description with the latest changes.
Apologies for the confusion.

@bitwalker bitwalker merged commit dbc6611 into 0xMiden:next Mar 30, 2026
12 checks passed
@lima-limon-inc
Copy link
Copy Markdown
Contributor Author

Awesome 😊

I'll know move to #876

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