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

Feat/rpc tests #1202

Merged
merged 26 commits into from
Mar 1, 2024
Merged

Feat/rpc tests #1202

merged 26 commits into from
Mar 1, 2024

Conversation

willemneal
Copy link
Member

What

Adds NetworkRunnable trait so that commands that need the RPC can have a common run function.

Adds to the bindings action to test RPC.

Why

[TODO: Why this change is being made. Include any context required to understand the why.]

Known limitations

[TODO or N/A]

@willemneal willemneal marked this pull request as ready for review February 15, 2024 20:09
@janewang
Copy link
Contributor

Blocked. Needs on help on this failure: https://github.com/stellar/soroban-tools/actions/runs/7921583339/job/21627229312?pr=1202 and also which quickstart to point to

@willemneal willemneal force-pushed the feat/rpc_tests branch 2 times, most recently from e465789 to 82a317e Compare February 21, 2024 20:17
@janewang
Copy link
Contributor

Still blocked. Tests are flaky. This blocks #1106

@willemneal
Copy link
Member Author

I was able to fix the rpc tests with some sleeps (which suggests a concurrency issue), but it seems that I shouldn't use async traits in one of the msrv tests because it's a recent addition to rust. Is this an issue? I could probably use the crate, but last time I tried (a year ago) I ran into some issues.
@leighmcculloch

@willemneal willemneal force-pushed the feat/rpc_tests branch 3 times, most recently from 59ece7d to f390d56 Compare February 28, 2024 13:59
@willemneal willemneal enabled auto-merge (squash) February 28, 2024 18:53
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

It's not ideal to keep those sleeps in, but also don't want to block us moving forward with this and getting this committed. I do think we need to get to the bottom of why the sleeps are needed though.

cmd/crates/soroban-test/src/lib.rs Show resolved Hide resolved
@willemneal willemneal merged commit 4b3fac8 into stellar:main Mar 1, 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
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants