Skip to content

Commit

Permalink
Fix up job object options for unit tests (microsoft#1335)
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 authored Mar 24, 2022
1 parent d33801f commit cc9e1f8
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 25 deletions.
5 changes: 2 additions & 3 deletions 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
32 changes: 10 additions & 22 deletions 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 @@ -263,12 +254,9 @@ 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)
// This test verifies that job.Pids() returns the right info and works with > 1
// process.
job, err := Create(context.Background(), nil)
if err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit cc9e1f8

Please sign in to comment.