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.

Sign up for GitHub

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

Jump to bottom

Soroban upgrades for simulation tests #4413

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Soroban upgrades for simulation tests #4413

merged 1 commit into from
Aug 30, 2024

Conversation

ThomasBrady
Copy link
Contributor

No description provided.

@ThomasBrady ThomasBrady changed the title DO NOTE MERGE Broken upgrade settings for tests DO NOT MERGE Broken upgrade settings for tests Aug 2, 2024
@ThomasBrady ThomasBrady changed the title DO NOT MERGE Broken upgrade settings for tests Soroban upgrades for simulation tests Aug 12, 2024
marta-lokhova
marta-lokhova requested changes Aug 21, 2024
View reviewed changes
src/test/TestUtils.cpp Show resolved Hide resolved
src/test/TestUtils.cpp Outdated Show resolved Hide resolved
src/test/TestUtils.cpp Outdated Show resolved Hide resolved
@marta-lokhova
Copy link
Contributor

@ThomasBrady what other tests are failing?

@ThomasBrady
Copy link
Contributor Author

ThomasBrady commented Aug 22, 2024 •
edited
Loading

@ThomasBrady what other tests are failing?

Thanks for the fix! I saw the residual carryOver but I didn't think of just clearing it 😅 .
I think they are all passing now, I fixed two other test cases today (generate soroban load, overlay flow control) as well as reading the config from the db. Running a full make check to confirm, then I'll push the update.

@marta-lokhova
Copy link
Contributor

@ThomasBrady looks like CI is failing?

@ThomasBrady
Copy link
Contributor Author

@ThomasBrady looks like CI is failing?

Formatting issues 🙃 re-running now 🙏

@marta-lokhova
Copy link
Contributor

@ThomasBrady thanks for the updates! Could we also confirm that acceptance tests pass? You can run [acceptance] tag to verify that.

@ThomasBrady
Copy link
Contributor Author

@ThomasBrady thanks for the updates! Could we also confirm that acceptance tests pass? You can run [acceptance] tag to verify that.

Can confirm now, acceptance tests pass.

marta-lokhova
marta-lokhova reviewed Aug 28, 2024
View reviewed changes
Copy link
Contributor

@marta-lokhova marta-lokhova left a comment

Choose a reason for hiding this comment

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

thanks, looks good overall! I left a few comments regarding code cleanup.

src/herder/test/HerderTests.cpp Outdated Show resolved Hide resolved
src/herder/test/HerderTests.cpp Outdated Show resolved Hide resolved
src/herder/test/HerderTests.cpp Outdated Show resolved Hide resolved
src/overlay/test/OverlayTests.cpp Outdated Show resolved Hide resolved
src/overlay/test/OverlayTests.cpp Outdated Show resolved Hide resolved
src/test/TestUtils.cpp Show resolved Hide resolved
src/test/TestUtils.cpp Outdated Show resolved Hide resolved
src/test/TestUtils.h Outdated Show resolved Hide resolved
src/test/TestUtils.cpp Outdated Show resolved Hide resolved
src/herder/test/HerderTests.cpp Outdated Show resolved Hide resolved
@ThomasBrady ThomasBrady force-pushed the upgrades-for-tests branch 2 times, most recently from 6374342 to 2a4018a Compare August 29, 2024 00:28
@ThomasBrady ThomasBrady mentioned this pull request Aug 29, 2024
Open
@marta-lokhova marta-lokhova added this pull request to the merge queue Aug 30, 2024
Merged via the queue into stellar:master with commit 62313d8 Aug 30, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marta-lokhova marta-lokhova marta-lokhova approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ThomasBrady @marta-lokhova