Skip to content
This repository has been archived by the owner on Dec 13, 2021. It is now read-only.

cli: add a new option -cmd option for executing given command to proc… #151

Merged
merged 1 commit into from
May 26, 2016
Merged

Conversation

mitake
Copy link
Contributor

@mitake mitake commented May 26, 2016

…ess inspector

This commit adds a new option -cmd to process inspector. With this
option, we don't need to start execution of target before
inspection. Below is an example:
$ sudo nmz inspectors proc -watch-interval 100ms -autopilot
dirichlet.toml -cmd "./integration.test -test.v -test.run TestIssue2746$ -test.count 1000"

@mitake
Copy link
Contributor Author

mitake commented May 26, 2016

PTAL @AkihiroSuda

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 41.703% when pulling f0f9858 on mitake:proc-exec into 8447a1a on osrg:master.

@AkihiroSuda
Copy link
Member

@mitake Thank you a lot and LGTM.

How about making the cmd to a non-option argument?

e.g.

$ sudo nmz inspectors proc -watch-interval 100ms -autopilot\
dirichlet.toml -- ./integration.test -test.v -test.run TestIssue2746\$ -test.count 1000

log.Critical("failed to StartProcess: %s", err)
return 1
}
procState, err := proc.Wait()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait() blocks until the process exits. https://golang.org/pkg/os/#Process.Wait
So you don't need this Wait()

@mitake
Copy link
Contributor Author

mitake commented May 26, 2016

@AkihiroSuda thanks for your comment. I updated this PR, PTAL.

The updated version has a mechanism for detecting end of target process.

return 1
}

pid = cmd.Process.Pid
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pid seems never used because it is overwritten in L96

@mitake
Copy link
Contributor Author

mitake commented May 26, 2016

@AkihiroSuda updated for the latest comments, thanks for pointing. PTAL.

@AkihiroSuda
Copy link
Member

@mitake LGTM, thanks!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 41.6% when pulling c1f0915 on mitake:proc-exec into 8447a1a on osrg:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 41.6% when pulling c1f0915 on mitake:proc-exec into 8447a1a on osrg:master.

@AkihiroSuda
Copy link
Member

AkihiroSuda commented May 26, 2016

but CI is failing.. https://travis-ci.org/osrg/namazu/builds/133052785

# github.com/osrg/namazu/nmz/inspector/proc
nmz/inspector/proc/proc_test.go:48: not enough arguments in call to insp.Serve

…ess inspector

This commit adds a new option -cmd to process inspector. With this
option, we don't need to start execution of target before
inspection. Below is an example:
$ sudo nmz inspectors proc -watch-interval 100ms -autopilot\
 dirichlet.toml  -cmd "./integration.test -test.v -test.run TestIssue2746\$ -test.count 1000"
@mitake
Copy link
Contributor Author

mitake commented May 26, 2016

updated for the CI problem

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 41.6% when pulling c1f0915 on mitake:proc-exec into 8447a1a on osrg:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 41.569% when pulling c2dfbeb on mitake:proc-exec into 8447a1a on osrg:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 41.612% when pulling 2778d5e on mitake:proc-exec into 8447a1a on osrg:master.

@AkihiroSuda
Copy link
Member

merging, thank you a lot!

@AkihiroSuda AkihiroSuda merged commit 89dea61 into osrg:master May 26, 2016
@mitake mitake deleted the proc-exec branch May 26, 2016 12:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants