Skip to content

Commit 7741ebe

Browse files
authored
Do not sortdesc by default in conductor + CLI test fix (#121)
Also fix the CLI tests, ensuring the application is properly shut down, before moving on: this would cause the subsequent admin server tests to fail. This started happening after I re-ordered the test suites.
1 parent 468e579 commit 7741ebe

File tree

4 files changed

+82
-94
lines changed

4 files changed

+82
-94
lines changed

cmd/dbos/cli_integration_test.go

Lines changed: 53 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package main
22

33
import (
4-
"bytes"
54
"context"
65
"database/sql"
76
_ "embed"
@@ -14,6 +13,7 @@ import (
1413
"os/exec"
1514
"path/filepath"
1615
"runtime"
16+
"syscall"
1717
"testing"
1818
"time"
1919

@@ -96,17 +96,28 @@ func TestCLIWorkflow(t *testing.T) {
9696
testProjectInitialization(t, cliPath)
9797
})
9898

99-
t.Run("ApplicationLifecycle", func(t *testing.T) {
100-
cmd := testApplicationLifecycle(t, cliPath)
101-
t.Cleanup(func() {
102-
if cmd.Process != nil {
103-
/*
104-
fmt.Println(cmd.Stderr)
105-
fmt.Println(cmd.Stdout)
106-
*/
107-
cmd.Process.Kill()
108-
}
109-
})
99+
// Start a test application using dbos start
100+
cmd := exec.CommandContext(context.Background(), cliPath, "start")
101+
cmd.Env = append(os.Environ(), "DBOS_SYSTEM_DATABASE_URL="+getDatabaseURL())
102+
err = cmd.Start()
103+
require.NoError(t, err, "Failed to start application")
104+
// Wait for server to be ready
105+
require.Eventually(t, func() bool {
106+
resp, err := http.Get("http://localhost:" + testServerPort)
107+
if err != nil {
108+
return false
109+
}
110+
resp.Body.Close()
111+
return resp.StatusCode == http.StatusOK
112+
}, 10*time.Second, 500*time.Millisecond, "Server should start within 10 seconds")
113+
114+
t.Cleanup(func() {
115+
fmt.Printf("Cleaning up application process %d\n", cmd.Process.Pid)
116+
// fmt.Println(cmd.Stderr)
117+
// fmt.Println(cmd.Stdout)
118+
err := syscall.Kill(cmd.Process.Pid, syscall.SIGTERM)
119+
require.NoError(t, err, "Failed to send interrupt signal to application process")
120+
_ = cmd.Wait()
110121
})
111122

112123
t.Run("WorkflowCommands", func(t *testing.T) {
@@ -120,7 +131,6 @@ func TestCLIWorkflow(t *testing.T) {
120131

121132
// testProjectInitialization verifies project initialization
122133
func testProjectInitialization(t *testing.T, cliPath string) {
123-
124134
// Initialize project
125135
cmd := exec.Command(cliPath, "init", testProjectName)
126136
output, err := cmd.CombinedOutput()
@@ -164,71 +174,6 @@ func testProjectInitialization(t *testing.T, cliPath string) {
164174
require.NoError(t, err, "go mod tidy failed: %s", string(modOutput))
165175
}
166176

167-
// testApplicationLifecycle starts the application and triggers workflows
168-
func testApplicationLifecycle(t *testing.T, cliPath string) *exec.Cmd {
169-
// Should already be in project directory from previous test
170-
171-
// Start the application in background
172-
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
173-
defer cancel()
174-
175-
cmd := exec.CommandContext(ctx, cliPath, "start")
176-
cmd.Env = append(os.Environ(), "DBOS_SYSTEM_DATABASE_URL="+getDatabaseURL())
177-
178-
// Capture output for debugging
179-
var stdout, stderr bytes.Buffer
180-
cmd.Stdout = &stdout
181-
cmd.Stderr = &stderr
182-
183-
err := cmd.Start()
184-
require.NoError(t, err, "Failed to start application")
185-
186-
// Wait for server to be ready
187-
require.Eventually(t, func() bool {
188-
resp, err := http.Get("http://localhost:" + testServerPort)
189-
if err != nil {
190-
return false
191-
}
192-
resp.Body.Close()
193-
return resp.StatusCode == http.StatusOK
194-
}, 10*time.Second, 500*time.Millisecond, "Server should start within 10 seconds")
195-
196-
// Trigger workflows via HTTP endpoints
197-
t.Run("TriggerExampleWorkflow", func(t *testing.T) {
198-
resp, err := http.Get("http://localhost:" + testServerPort + "/workflow")
199-
require.NoError(t, err, "Failed to trigger workflow")
200-
defer resp.Body.Close()
201-
202-
body, err := io.ReadAll(resp.Body)
203-
require.NoError(t, err)
204-
205-
assert.Equal(t, http.StatusOK, resp.StatusCode, "Workflow endpoint should return 200")
206-
assert.Contains(t, string(body), "Workflow result", "Should contain workflow result")
207-
})
208-
209-
t.Run("TriggerQueueWorkflow", func(t *testing.T) {
210-
resp, err := http.Get("http://localhost:" + testServerPort + "/queue")
211-
require.NoError(t, err, "Failed to trigger queue workflow")
212-
defer resp.Body.Close()
213-
214-
body, err := io.ReadAll(resp.Body)
215-
require.NoError(t, err)
216-
217-
assert.Equal(t, http.StatusOK, resp.StatusCode, "Queue endpoint should return 200")
218-
219-
// Parse JSON response to get workflow ID
220-
var response map[string]string
221-
err = json.Unmarshal(body, &response)
222-
require.NoError(t, err, "Should be valid JSON response")
223-
224-
workflowID, exists := response["workflow_id"]
225-
assert.True(t, exists, "Response should contain workflow_id")
226-
assert.NotEmpty(t, workflowID, "Workflow ID should not be empty")
227-
})
228-
229-
return cmd
230-
}
231-
232177
// testWorkflowCommands comprehensively tests all workflow CLI commands
233178
func testWorkflowCommands(t *testing.T, cliPath string) {
234179

@@ -256,8 +201,29 @@ func testWorkflowCommands(t *testing.T, cliPath string) {
256201
// testListWorkflows tests various workflow listing scenarios
257202
func testListWorkflows(t *testing.T, cliPath string) {
258203
// Create some test workflows first to ensure we have data to filter
259-
// The previous test functions have already created workflows that we can query
204+
resp, err := http.Get("http://localhost:" + testServerPort + "/workflow")
205+
require.NoError(t, err, "Failed to trigger workflow")
206+
defer resp.Body.Close()
207+
body, err := io.ReadAll(resp.Body)
208+
require.NoError(t, err)
209+
assert.Equal(t, http.StatusOK, resp.StatusCode, "Workflow endpoint should return 200")
210+
assert.Contains(t, string(body), "Workflow result", "Should contain workflow result")
211+
212+
resp, err = http.Get("http://localhost:" + testServerPort + "/queue")
213+
require.NoError(t, err, "Failed to trigger queue workflow")
214+
defer resp.Body.Close()
215+
body, err = io.ReadAll(resp.Body)
216+
require.NoError(t, err)
217+
assert.Equal(t, http.StatusOK, resp.StatusCode, "Queue endpoint should return 200")
218+
219+
// Parse JSON response to get workflow ID
220+
var response map[string]string
221+
err = json.Unmarshal(body, &response)
222+
require.NoError(t, err, "Should be valid JSON response")
260223

224+
workflowID, exists := response["workflow_id"]
225+
assert.True(t, exists, "Response should contain workflow_id")
226+
assert.NotEmpty(t, workflowID, "Workflow ID should not be empty")
261227
// Get the current time for time-based filtering
262228
currentTime := time.Now()
263229

@@ -724,14 +690,14 @@ func buildCLI(t *testing.T) string {
724690
// Build output path in the cmd directory
725691
cliPath := filepath.Join(cmdDir, "dbos-cli-test")
726692

727-
// Check if already built
728-
if _, err := os.Stat(cliPath); os.IsNotExist(err) {
729-
// Build the CLI from the cmd directory
730-
buildCmd := exec.Command("go", "build", "-o", "dbos-cli-test", ".")
731-
buildCmd.Dir = cmdDir
732-
buildOutput, buildErr := buildCmd.CombinedOutput()
733-
require.NoError(t, buildErr, "Failed to build CLI: %s", string(buildOutput))
734-
}
693+
// Delete any existing binary before building
694+
os.Remove(cliPath)
695+
696+
// Build the CLI from the cmd directory
697+
buildCmd := exec.Command("go", "build", "-o", "dbos-cli-test", ".")
698+
buildCmd.Dir = cmdDir
699+
buildOutput, buildErr := buildCmd.CombinedOutput()
700+
require.NoError(t, buildErr, "Failed to build CLI: %s", string(buildOutput))
735701

736702
// Return absolute path
737703
absPath, err := filepath.Abs(cliPath)

cmd/dbos/cli_test_app.go.test

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"fmt"
77
"net/http"
88
"os"
9+
"os/signal"
10+
"syscall"
911
"time"
1012

1113
"github.com/dbos-inc/dbos-transact-golang/dbos"
@@ -68,7 +70,7 @@ func QueueWorkflow(ctx dbos.DBOSContext, _ string) (string, error) {
6870
}
6971
handles[i] = handle
7072
}
71-
time.Sleep(10 * time.Second) // give some time for our tests to do wf management
73+
time.Sleep(10 * time.Second) // give some time for our tests to do wf management
7274
return fmt.Sprintf("Successfully enqueued %d steps", len(handles)), nil
7375
}
7476

@@ -114,6 +116,16 @@ func main() {
114116
http.HandleFunc("/queue", queueHandler)
115117
http.HandleFunc("/", healthHandler)
116118

119+
// Set up signal handling
120+
sigChan := make(chan os.Signal, 1)
121+
signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM)
122+
123+
go func() {
124+
<-sigChan
125+
fmt.Println("Received interrupt signal, shutting down...")
126+
os.Exit(0)
127+
}()
128+
117129
fmt.Println("Server starting on http://localhost:8080")
118130
err = http.ListenAndServe(":8080", nil)
119131
if err != nil {
@@ -156,5 +168,5 @@ func queueHandler(w http.ResponseWriter, r *http.Request) {
156168
}
157169

158170
func healthHandler(w http.ResponseWriter, r *http.Request) {
159-
fmt.Fprintf(w, "healthy")
160-
}
171+
fmt.Fprintf(w, "healthy")
172+
}

cmd/dbos/start.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"os/signal"
88
"runtime"
99
"syscall"
10+
"time"
1011

1112
"github.com/spf13/cobra"
1213
)
@@ -73,7 +74,7 @@ func runStart(cmd *cobra.Command, args []string) error {
7374
return fmt.Errorf("command failed: %w", err)
7475
}
7576
case sig := <-sigChan:
76-
logger.Info("Received signal, stopping...", "signal", sig)
77+
logger.Info("Received signal, stopping...", "signal", sig.String())
7778

7879
// Kill the process group on Unix-like systems
7980
if runtime.GOOS != "windows" {
@@ -90,6 +91,11 @@ func runStart(cmd *cobra.Command, args []string) error {
9091
if runtime.GOOS != "windows" {
9192
syscall.Kill(-process.Process.Pid, syscall.SIGKILL)
9293
}
94+
case <-time.After(10 * time.Second):
95+
// Force kill after timeout
96+
if runtime.GOOS != "windows" {
97+
syscall.Kill(-process.Process.Pid, syscall.SIGKILL)
98+
}
9399
}
94100

95101
os.Exit(0)

dbos/conductor.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,9 @@ func (c *Conductor) handleListWorkflowsRequest(data []byte, requestID string) er
562562
var opts []ListWorkflowsOption
563563
opts = append(opts, WithLoadInput(req.Body.LoadInput))
564564
opts = append(opts, WithLoadOutput(req.Body.LoadOutput))
565-
opts = append(opts, WithSortDesc())
565+
if req.Body.SortDesc {
566+
opts = append(opts, WithSortDesc())
567+
}
566568
if len(req.Body.WorkflowUUIDs) > 0 {
567569
opts = append(opts, WithWorkflowIDs(req.Body.WorkflowUUIDs))
568570
}
@@ -638,8 +640,7 @@ func (c *Conductor) handleListQueuedWorkflowsRequest(data []byte, requestID stri
638640
var opts []ListWorkflowsOption
639641
opts = append(opts, WithLoadInput(req.Body.LoadInput))
640642
opts = append(opts, WithLoadOutput(false)) // Don't load output for queued workflows
641-
opts = append(opts, WithSortDesc())
642-
opts = append(opts, WithQueuesOnly()) // Only include workflows that are in queues
643+
opts = append(opts, WithQueuesOnly()) // Only include workflows that are in queues
643644

644645
// Add status filter for queued workflows
645646
queuedStatuses := make([]WorkflowStatusType, 0)
@@ -656,6 +657,9 @@ func (c *Conductor) handleListQueuedWorkflowsRequest(data []byte, requestID stri
656657
}
657658
opts = append(opts, WithStatus(queuedStatuses))
658659

660+
if req.Body.SortDesc {
661+
opts = append(opts, WithSortDesc())
662+
}
659663
if req.Body.WorkflowName != nil {
660664
opts = append(opts, WithName(*req.Body.WorkflowName))
661665
}

0 commit comments

Comments
 (0)