-
Notifications
You must be signed in to change notification settings - Fork 141
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
validation: implement PosixHooksCalledInOrder test #577
Conversation
|
When |
I think in the order tests, we have the assumption: every hook ('echo') will be executed collect, we just test the 'order'. |
6f24a7b
to
c74cd8a
Compare
updated, and I think hook.go would be better. |
validation/hooks.go
Outdated
"sh", "-c", fmt.Sprintf("echo 'pre-start1 called' >> %s", output), | ||
}, | ||
} | ||
g.AddPreStartHook(prestart1) |
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.
No need for a prestart1
temporary variable, you can just use:
err := g.AddPreStartHook(rspec.Hook{
…
})
if err != nil {
return err
}
That also checks this call's returned error, which you're currently ignoring.
validation/hooks.go
Outdated
g := util.GetDefaultGenerator() | ||
output = filepath.Join(r.BundleDir, g.Spec().Root.Path, "output") | ||
prestart1 := rspec.Hook{ | ||
Path: filepath.Join(r.BundleDir, g.Spec().Root.Path, "/bin/sh"), |
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.
You use this Join
a lot in this test. Maybe store it in a temporary variable:
shPath := filepath.Join(r.BundleDir, g.Spec().Root.Path, "/bin/sh")
…
Path: shPath,
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, thanks.
Signed-off-by: Zhou Hao <[email protected]>
c74cd8a
to
4947839
Compare
Signed-off-by: Zhou Hao [email protected]