Make test wasms build more consistently#1576
Merged
leighmcculloch merged 42 commits intomainfrom Sep 30, 2025
Merged
Conversation
…d-test and update download-artifact version
graydon
approved these changes
Sep 29, 2025
This was referenced Sep 30, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 30, 2025
### What Install the msrv ahead of using it as an override. Remove making warnings declined in the Makefile. Do not rebuild test wasms when running tests. Change the default target to just run tests. ### Why Few things I wish I had done slightly differently in #1576. Warnings are enabled in the makefile, but that needs to be set only in CI, and only on the msrv now that the CI uses the makefile. Right now warnings are enabled for both msrv and latest, when they shouldn't be. Having warnings as errors off on local is good. The tests run by the makefile always build the test wasms, but in CI that doesn't need to happen as the test wasms are injected as an artifact. The default target currently calls the check target, which no longer exists.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Remove the rssdkver (Rust SDK Version) meta entry from the builds of the test wasms in this repository.
And build the test wasms once with the minimum supported rust version (msrv) and use that build for test runs.
Why
When building contracts with the soroban-sdk imported, the Rust SDK Version is embedded in a meta entry. For the test wasms in this repository this makes their builds unstable, and inconsistent. Every commit on the repository results in a slightly different build given that the revision is included and is forever changing. This makes it challenging to use the wasms in some places where the hash or the raw bytes get captured, such as test snapshots, and soon expanded generated code (#1572).
The simplest way to remove this problem is to remove the meta entry just when building the local test vectors.
Each version of rust can also cause differences in the wasm build, so for tests, only testing against the wasms built with the msrv keeps the wasm binaries stable for the longest period of time. This change required some refactoring of the ci process, to separate building and testing. That refactor also moved us away from using a separate set of commands in ci vs locally, now ci just runs make commands.
The local Makefile is updated to build the wasms with the msrv as well for convenience.