Skip to content

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

Closed
jsvisa wants to merge 1 commit into
ethereum:masterfrom
jsvisa:fix_rpc_ipc_windows
Closed

rpc: url.Parse can't parse filepath in windows#17151
jsvisa wants to merge 1 commit into
ethereum:masterfrom
jsvisa:fix_rpc_ipc_windows

Conversation

@jsvisa
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

The test is supposedly fixed already on master.

@karalabe karalabe closed this Jul 10, 2018
@nonsense
Copy link
Copy Markdown
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