Skip to content

Commit

Permalink
Fix up job object options for unit tests
Browse files Browse the repository at this point in the history
Most of the `jobobject` package tests we have ask for options that aren't
actually needed/used. This change makes it so that any test that doesn't need
a named job object doesn't ask for one and any test that doesn't plan on
using the iocp messages doesn't flip the notifications field either. This
wasn't causing any issues, but it's probably best to filter down what's
being tested to only what's needed.

Additionally fixes TestExecsWithJob that used log.Fatal instead of t.Fatal
in the test.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
  • Loading branch information
dcantah committed Mar 23, 2022
1 parent a2ed14c commit 0749a73
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 24 deletions.
5 changes: 2 additions & 3 deletions internal/exec/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package exec
import (
"context"
"io"
"log"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -127,9 +126,9 @@ func TestExecStdinPowershell(t *testing.T) {

func TestExecsWithJob(t *testing.T) {
// Test that we can assign processes to a job object at creation time.
job, err := jobobject.Create(context.Background(), &jobobject.Options{Name: "test"})
job, err := jobobject.Create(context.Background(), nil)
if err != nil {
log.Fatal(err)
t.Fatal(err)
}
defer job.Close()

Expand Down
29 changes: 8 additions & 21 deletions internal/jobobject/jobobject_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ func TestJobCreateAndOpen(t *testing.T) {
ctx = context.Background()
options = &Options{Name: "test"}
)

jobCreate, err := Create(ctx, options)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -67,11 +66,7 @@ func createProcsAndAssign(num int, job *JobObject) (_ []*exec.Cmd, err error) {
}

func TestSetTerminateOnLastHandleClose(t *testing.T) {
options := &Options{
Name: "test",
Notifications: true,
}
job, err := Create(context.Background(), options)
job, err := Create(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -99,7 +94,7 @@ func TestSetTerminateOnLastHandleClose(t *testing.T) {
if !procs[0].ProcessState.Exited() {
errCh <- errors.New("process should have exited after closing job handle")
}
errCh <- nil
close(errCh)
}()

select {
Expand All @@ -116,11 +111,7 @@ func TestSetTerminateOnLastHandleClose(t *testing.T) {
func TestSetMultipleExtendedLimits(t *testing.T) {
// Tests setting two different properties on the job that modify
// JOBOBJECT_EXTENDED_LIMIT_INFORMATION
options := &Options{
Name: "test",
Notifications: true,
}
job, err := Create(context.Background(), options)
job, err := Create(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -158,7 +149,6 @@ func TestNoMoreProcessesMessageKill(t *testing.T) {
// Test that we receive the no more processes in job message after killing all of
// the processes in the job.
options := &Options{
Name: "test",
Notifications: true,
}
job, err := Create(context.Background(), options)
Expand Down Expand Up @@ -192,7 +182,8 @@ func TestNoMoreProcessesMessageKill(t *testing.T) {

switch notif.(type) {
case MsgAllProcessesExited:
errCh <- nil
close(errCh)
return
case MsgUnimplemented:
default:
}
Expand All @@ -213,7 +204,6 @@ func TestNoMoreProcessesMessageTerminate(t *testing.T) {
// Test that we receive the no more processes in job message after terminating the
// job (terminates every process in the job).
options := &Options{
Name: "test",
Notifications: true,
}
job, err := Create(context.Background(), options)
Expand Down Expand Up @@ -245,7 +235,8 @@ func TestNoMoreProcessesMessageTerminate(t *testing.T) {

switch notif.(type) {
case MsgAllProcessesExited:
errCh <- nil
close(errCh)
return
case MsgUnimplemented:
default:
}
Expand All @@ -264,11 +255,7 @@ func TestNoMoreProcessesMessageTerminate(t *testing.T) {

func TestVerifyPidCount(t *testing.T) {
// This test verifies that job.Pids() returns the right info and works with > 1 process.
options := &Options{
Name: "test",
Notifications: true,
}
job, err := Create(context.Background(), options)
job, err := Create(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit 0749a73

Please sign in to comment.