-
Notifications
You must be signed in to change notification settings - Fork 21
feat: Link the instrumented binary and run it and further check output content #171
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
base: main
Are you sure you want to change the base?
Conversation
46710d8 to
fb0ccd2
Compare
…t content Signed-off-by: yuluo-yx <[email protected]>
fb0ccd2 to
10a4443
Compare
| } | ||
|
|
||
| // TestInstrumentWithHooks tests FuncRules and StructRules instrumentation | ||
| func TestInstrumentWithHooks(t *testing.T) { |
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.
Could you please clarify the rationale for splitting this into two tests? I was thinking we could handle the entire process—injection, compilation, linking, and result verification—within a single test.
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.
|
Thank you for your contribution. I'm sorry, but let's put this patch on hold for the time being. The workflow I was expecting is: compile test.a + compile hook.a => link test.a hook.a => a.out => exec a.out. This is different from the current approach of using go build . to build the program and capture its output. That process is essentially an integration test, which we already have covered. Perhaps there's a fundamental issue with this requirement itself. The linking step (link test.a hook.a) cannot proceed without runtime.a. However, we are finding it difficult to get ahold of runtime.a in a reliable way. Since Go 1.20, modules cause runtime.a to be placed in the build cache (the go-build directory, i.e., GOCACHE) instead of the old $GOPATH/pkg location. This change further complicates the task. |
instrument_test.go#104