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: simple IPC test #65

Merged
merged 18 commits into from
Dec 8, 2023
Merged

feat: simple IPC test #65

merged 18 commits into from
Dec 8, 2023

Conversation

Evalir
Copy link
Contributor

@Evalir Evalir commented Dec 6, 2023

Motivation

IPC didn't have any tests for making a request

Solution

Adds a simple sanity test using the geth binary (we can switch it out for a mock later)

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Comment on lines 23 to 25
install_geth &
g=$!
wait $g $!
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
install_geth &
g=$!
wait $g $!
install_geth

Copy link
Member

Choose a reason for hiding this comment

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

i would really love not to install geth in our tests, is there an expedient way we can do this with a mock? 😮‍💨

.github/scripts/install_test_binaries.sh Outdated Show resolved Hide resolved
crates/rpc-client/Cargo.toml Outdated Show resolved Hide resolved
crates/rpc-client/Cargo.toml Show resolved Hide resolved
Copy link
Contributor Author

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

@prestwich seems like the ipc client doesn't read the stream at all and it times out? https://github.com/alloy-rs/alloy/actions/runs/7119215013/job/19383813553

Copy link
Contributor Author

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

test passes now @prestwich — any chance we can merge like this and clean up geth later?

@Evalir Evalir merged commit 6005c4c into prestwich/interprocess Dec 8, 2023
16 checks passed
Evalir added a commit that referenced this pull request Dec 8, 2023
* feature: interprocess-based IPC

* cleanup: tracing and flow

* feat: log buffer contents

* chore: do not run wasm check on alloy-transport-ipc

* chore: add cc0-1.0 for to_method

* fix: buffer properly

* feature: some rexports and client impl

* feature: mock ipc

* docs: some of them

* lint: clippy

* nit: newline

* feat: close file so other processes can access it

* feat: simple `IPC` test (#65)

* chore: add basic ipc test

* install test binaries

* feat: ipc connection on builder if ipc feat is enabled

* chore: remove, better done separately

* review comments

* review comments

* chore: remove unused crates for now

* add to dev deps

* chore: add instructions for repro on ipc test

* chore: re-add normal test

* clippy

* chore: change test to use mock ipc

* add mock

* no sleep till brooklyn

* chore: revert to geth impl for now

---------

Co-authored-by: Enrique Ortiz <[email protected]>
@DaniPopes DaniPopes deleted the evalir/ipc-test branch January 15, 2024 18:11
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