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

rpc: url.Parse can't parse filepath in windows #17151

Closed
wants to merge 1 commit into from

Conversation

jsvisa
Copy link
Contributor

@jsvisa jsvisa commented Jul 9, 2018

As #17141 said, CI failed in Windowss after PR #17041 merged, because #17041 introduced a new test case TestNewSwarm in swarm_test.go#L38, diff. AFAIK, this bug exists for a long time, long before PR #17041, and it happened in that PR.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

I don't really follow why this change solves a problem really. On Windows, IPC starts with \\ as far as I know. What does this PR meant to solve?

@jsvisa
Copy link
Contributor Author

jsvisa commented Jul 10, 2018

@karalabe as https://ci.appveyor.com/project/ethereum/go-ethereum/build/master.7006/job/tocjijjprtmv4tac#L1227 said:

swarm_test.go:175: error connecting to SWAP API C:\Users\appveyor\AppData\Local\Temp\1\swarm837570684/TestSwarm.ipc: no known transport for URL scheme "c"

url.Parse parse "C:\Users\appveyor\AppData\Local\Temp\1\swarm837570684/TestSwarm.ipc" scheme to "c"

@karalabe
Copy link
Member

C:\Users\appveyor\AppData\Local\Temp\1\swarm837570684/TestSwarm.ipc is not a valid IPC path on windows. The test needs to be fixed.

@karalabe
Copy link
Member

The test is supposedly fixed already on master.

@karalabe karalabe closed this Jul 10, 2018
@nonsense
Copy link
Member

nonsense commented Jul 10, 2018

Fixed as part of b3711af by @janos

@jsvisa jsvisa deleted the fix_rpc_ipc_windows branch July 23, 2018 10:43
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