-
Notifications
You must be signed in to change notification settings - Fork 172
Fix tests on MacOS (darwin) #498
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
Conversation
Error is "The ipc endpoint is longer than %d characters."
cmd/geth/consolecmd_cg_test.go
Outdated
| } | ||
| for i, c := range cases { | ||
| t.Run(fmt.Sprintf("TestGethStartupLogs/%d: %v", i, c.flags), func(t *testing.T) { | ||
| // fix for darwin, where long test names fail with "The ipc endpoint is longer than %d characters." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@meowsbits another way is to change the test name only if rpc.max_path_size < len([]byte(strings.Join(c.flags, " ")))), for this we have to export the max_path_size in rpc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://unix.stackexchange.com/questions/367008/why-is-socket-path-length-limited-to-a-hundred-chars
If the error you're seeing is because of the IPC path length limit, then we can try to shorten that up by shortening the datadir path, since geth should use the default <datadir>/<alt chain?>/geth/geth.ipc.
But if the error is just because of the test names, I recommend reducing them to a simple Sprintf("%d", i) -- which seems like a reasonable thing to do anyway (shoving the flags in there was a bad idea).
I'm going to push a commit, see whatcha think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue was because of the IPC path, which was long as part of it was based on test name.
I am going to test your commits
…ix-m1-nil-pointer for fixing test issue
If IPC paths are too long, Unix systems will complain because there are limits, albeit inconsistent ones. So we try to keep IPC path names short (ie <100 chars). I removed all instances where flags were used in test names because - they cause issues with apple silicon M1, - and they are pointless I also removed the redundant subtest naming scheme which included the test name. Go includes the test name by default anyway. Date: 2022-08-29 09:21:05-05:00 Signed-off-by: meows <[email protected]>
Bumps dep to a tag version equivalent to the previously referenced, untagged version. Date: 2022-08-29 09:22:26-05:00 Signed-off-by: meows <[email protected]>
No description provided.