-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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] Fix ARM unit test #12639
base: main
Are you sure you want to change the base?
[chore] Fix ARM unit test #12639
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12639 +/- ##
=======================================
Coverage 91.57% 91.57%
=======================================
Files 483 483
Lines 26381 26381
=======================================
Hits 24158 24158
Misses 1762 1762
Partials 461 461 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@@ -1168,7 +1168,7 @@ func sendTestRequest(t *testing.T, gcs ClientConfig) (ptraceotlp.ExportResponse, | |||
|
|||
// tempSocketName provides a temporary Unix socket name for testing. | |||
func tempSocketName(t *testing.T) string { | |||
tmpfile, err := os.CreateTemp(t.TempDir(), "sock") |
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.
Why? Please add a comment why t.TempDir()
is not good.
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.
related to golang/go#71742?
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.
@bogdandrutu @codeboten I believe the issue stems from the path length limit for sockets on Unix systems. The os.TempDir()
function works because it generates a shorter file name compared to t.TempDir()
. This link might be helpful: https://unix.stackexchange.com/questions/367008/why-is-socket-path-length-limited-to-a-hundred-chars
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.
Please only comment what we discussed here because we will forget why we did this change.
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.
updated
@@ -1168,7 +1168,7 @@ func sendTestRequest(t *testing.T, gcs ClientConfig) (ptraceotlp.ExportResponse, | |||
|
|||
// tempSocketName provides a temporary Unix socket name for testing. | |||
func tempSocketName(t *testing.T) string { | |||
tmpfile, err := os.CreateTemp(t.TempDir(), "sock") |
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.
related to golang/go#71742?
Description
This PR fixes a test in ARM by using a shorter file name in socket (rolling back the behavior introduced in #12576 ). Seems that we ran into an issue similar to golang/go#6895
Link to tracking issue
n/a
Testing
n/a
Documentation
n/a