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

chore(general): add CI workflow for Windows + fix IPC test #642

Merged
merged 22 commits into from
May 1, 2024

Conversation

zerosnacks
Copy link
Member

@zerosnacks zerosnacks commented Apr 26, 2024

Motivation

Due to cross-platform related issues arising around IPC it makes sense to add a Windows target to the CI workflow to ensure cross-compatibility.

Previously failed to build and run for various reasons, one of them being the path to Geth not being defined correctly for the tests.

Related PR in core: alloy-rs/core#613

Solution

  • Implements windows-latest as test target, maintaining the existing test matrix
  • Fixes ability to locate Geth binary in Alloy tests on Windows (shell: bash)
  • Fixes broken IPC test, on Windows Geth's --ipcpath refers to the name of the ipc file, not the actual path as is the case on Unix. Modified the name to show the ability of spawning and referencing with a custom IPC path.
  • Disables IPC by default when spawning Geth to prevent the common Access Denied errors
  • Fixes dependency failure (missing alloy-transport-ws.workspace for ws)
  • Prefer serial as tag imported - leftover from Geth debugging but preferable in my opinion.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@zerosnacks
Copy link
Member Author

zerosnacks commented Apr 29, 2024

Reopening once I've fixed the run issues in my fork

@zerosnacks zerosnacks closed this Apr 29, 2024
@zerosnacks zerosnacks reopened this Apr 29, 2024
@zerosnacks zerosnacks marked this pull request as ready for review April 29, 2024 17:23
@zerosnacks zerosnacks requested a review from DaniPopes as a code owner April 29, 2024 17:23
@zerosnacks zerosnacks changed the title chore(general): add CI workflow for Windows chore(general): add CI workflow for Windows + fix IPC test Apr 30, 2024
@zerosnacks zerosnacks merged commit 5a5f29e into alloy-rs:main May 1, 2024
25 of 48 checks passed
ben186 pushed a commit to ben186/alloy that referenced this pull request Jul 27, 2024
)

* add windows workflow

* make sure it runs on the target

* export path after modification

* update workflow, set tracing for failing geth script

* check if geth is in path

* comment out other jobs

* attempt

* use bash shell

* works! error on access denied for ipc

* make sure Geth tests run serially to avoid ipc access denied

* disable ipc by default

* remove serial tag

* log path

* attempt fix by pointing towards Geths spawned IPC

* use geth port

* windows doesnt allow specific paths to ipc

* ipcpath on windows is just the name, not the path

* re-enable other jobs tasks

* clean up

* use custom ipc name to show working

* clean up
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.

4 participants