Skip to content
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

Testutils bug(s) #4489

Merged
merged 4 commits into from
Sep 27, 2024
Merged

Testutils bug(s) #4489

merged 4 commits into from
Sep 27, 2024

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Sep 27, 2024

This makes 3 changes:

  1. Stops passing --features testutils to soroban when building with BUILD_TESTS
  2. Starts passing it to librust_stellar_core.a (where it enables the side-by-side execution feature)
  3. Fixes a couple minor typos in the code that was gated by that

The key part here is the first; evidently having testutils turned on on soroban makes the cost structure observably different and thereby causes divergence during replay. This seems wrong to me, and something we should track down and eliminate, but in the meantime this at least (a) gets rid of that divergence and (b) does what we did before I accidentally started passing testutils in the recent soroban-submodule change (#4456)

@@ -8,12 +8,6 @@ include $(srcdir)/src.mk

noinst_HEADERS = $(SRC_H_FILES)

if BUILD_TESTS
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to remove this block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was accidental duplication while editing.

@sisuresh sisuresh enabled auto-merge September 27, 2024 01:39
@sisuresh sisuresh added this pull request to the merge queue Sep 27, 2024
Merged via the queue into stellar:master with commit 2d8d764 Sep 27, 2024
13 checks passed
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.

2 participants