-
Notifications
You must be signed in to change notification settings - Fork 5.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
Bugfix/procstat adds new procstat metric about pgrep search #4307
Conversation
plugins/inputs/procstat/pgrep.go
Outdated
if err != nil { | ||
return "", fmt.Errorf("Error running %s: %s", path, err) | ||
//return "", fmt.Errorf("Error running %s: %s", path, err) | ||
return "", fmt.Errorf("Error executing command: %v %v, err: %s", path, args, err) |
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.
Keep only the old error message.
@@ -369,3 +370,39 @@ func TestGather_cgroupPIDs(t *testing.T) { | |||
assert.Equal(t, []PID{1234, 5678}, pids) | |||
assert.Equal(t, td, tags["cgroup"]) | |||
} | |||
|
|||
func TestExitStatus(t *testing.T) { | |||
out, err := exec.Command("pgrep", "-G", "sys").Output() |
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.
This command could return different results on different systems, so it could create random failures, perhaps you can make the needed error by hand. Also, it tests a function in internal/internal.go
so it should be placed in internal/internal_test.go
|
||
func TestNoPgrepResults(t *testing.T) { | ||
p := Procstat{ | ||
PidFinder: "pgrep", |
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.
Same comment as above for this and the next test. You should be able to use testPgrep in this same file to fake the results.
|
||
func TestExitStatus(t *testing.T) { | ||
out, err := exec.Command("pgrep", "-G", "sys").Output() | ||
t.Logf("cmd out: %v", out) |
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.
Remove any logging in the test cases, use assertions to document results.
* origin: (39 commits) Document path tag in tail input Update changelog Added path tag to tail input plugin (influxdata#4292) Run windows tests with -short Fix postfix input handling of multi-level queues (influxdata#4333) Update changelog Add support for comma in logparser timestamp format (influxdata#4311) Update vendoring to dep from gdm (influxdata#4314) Update changelog Add new measurement with results of pgrep lookup to procstat input (influxdata#4307) Update changelog Add valuecounter aggregator plugin (influxdata#3523) Update changelog Update docker input documentation for container status Add container status tag to docker input (influxdata#4259) Drop CI support for Go 1.8 (influxdata#4309) Update changelog Fix selection of tags under nested objects in the JSON parser (influxdata#4284) Update changelog Add owner tag on partitions in burrow input (influxdata#4281) ...
Required for all PRs: