Skip to content
4 changes: 2 additions & 2 deletions manifests/charts/base/templates/agentcube-router.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ spec:
value: {{ .Values.redis.addr | quote }}
- name: REDIS_PASSWORD
value: {{ .Values.redis.password | quote }}
- name: WORKLOAD_MANAGER_ADDR
- name: WORKLOAD_MANAGER_URL
value: "http://workloadmanager.{{ .Release.Namespace }}.svc.cluster.local:{{ .Values.workloadmanager.service.port }}"
{{- with .Values.router.extraEnv }}
{{- toYaml . | nindent 12 }}
Expand Down Expand Up @@ -78,4 +78,4 @@ spec:
protocol: TCP
name: http
selector:
app: agentcube-router
app: agentcube-router
32 changes: 16 additions & 16 deletions pkg/router/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,24 @@ func (m *mockSessionManager) GetSandboxBySession(_ context.Context, _ string, _
func setupEnv() {
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
}

func teardownEnv() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}

func TestHandleHealth(t *testing.T) {
// Set required environment variables
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This block for setting and unsetting environment variables is repeated in multiple tests. To improve maintainability and reduce code duplication, you can use the existing setupEnv and teardownEnv helper functions.

Suggested change
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()
setupEnv()
defer teardownEnv()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@katara-Jayprakash pelease help fix this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i fixed them and additionally refactor the another testCases


config := &Config{
Expand Down Expand Up @@ -96,11 +96,11 @@ func TestHandleHealthLive(t *testing.T) {
// Set required environment variables
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

config := &Config{
Expand Down Expand Up @@ -130,11 +130,11 @@ func TestHandleHealthReady(t *testing.T) {
// Set required environment variables
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

tests := []struct {
Expand Down Expand Up @@ -248,11 +248,11 @@ func TestHandleInvoke_NoEntryPoints(t *testing.T) {
// Set required environment variables
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

config := &Config{
Expand Down Expand Up @@ -286,11 +286,11 @@ func TestHandleAgentInvoke(t *testing.T) {
// Set required environment variables
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

// Create a test HTTP server to act as the sandbox
Expand Down Expand Up @@ -355,11 +355,11 @@ func TestHandleCodeInterpreterInvoke(t *testing.T) {
// Set required environment variables
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

// Create a test HTTP server to act as the sandbox
Expand Down Expand Up @@ -457,11 +457,11 @@ func TestConcurrencyLimitMiddleware_Overload(t *testing.T) {
// Set required environment variables
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

config := &Config{
Expand Down
28 changes: 14 additions & 14 deletions pkg/router/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ func TestNewServer(t *testing.T) {
// Set required environment variables for tests
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This block for setting and unsetting environment variables is repeated across all tests in this file. To improve maintainability and reduce boilerplate, consider creating a helper function that sets the environment variables and uses t.Cleanup for teardown.

For example, you could add:

func setupTestEnv(t *testing.T) {
    t.Helper()
    os.Setenv("REDIS_ADDR", "localhost:6379")
    os.Setenv("REDIS_PASSWORD", "test-password")
    os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
    t.Cleanup(func() {
        os.Unsetenv("REDIS_ADDR")
        os.Unsetenv("REDIS_PASSWORD")
        os.Unsetenv("WORKLOAD_MANAGER_URL")
    })
}

And then call setupTestEnv(t) at the start of each test.

For a more direct improvement, you can replace the defer with t.Cleanup which is the modern, idiomatic way to handle test cleanup in Go.

Suggested change
// Set required environment variables for tests
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()
// Set required environment variables for tests
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
t.Cleanup(func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_URL")
})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed!


tests := []struct {
Expand Down Expand Up @@ -116,11 +116,11 @@ func TestServer_DefaultValues(t *testing.T) {
// Set required environment variables for tests
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

config := &Config{
Expand All @@ -143,11 +143,11 @@ func TestServer_ConcurrencyLimitMiddleware(t *testing.T) {
// Set required environment variables for tests
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

config := &Config{
Expand Down Expand Up @@ -176,11 +176,11 @@ func TestServer_SetupRoutes(t *testing.T) {
// Set required environment variables for tests
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

config := &Config{
Expand All @@ -205,11 +205,11 @@ func TestServer_StartContext(t *testing.T) {
// Set required environment variables for tests
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

config := &Config{
Expand Down Expand Up @@ -253,11 +253,11 @@ func TestServer_TLSConfiguration(t *testing.T) {
// Set required environment variables for tests
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

tests := []struct {
Expand Down Expand Up @@ -332,11 +332,11 @@ func TestServer_RedisIntegration(t *testing.T) {
// Set required environment variables for tests
os.Setenv("REDIS_ADDR", "localhost:6379")
os.Setenv("REDIS_PASSWORD", "test-password")
os.Setenv("WORKLOAD_MANAGER_ADDR", "http://localhost:8080")
os.Setenv("WORKLOAD_MANAGER_URL", "http://localhost:8080")
defer func() {
os.Unsetenv("REDIS_ADDR")
os.Unsetenv("REDIS_PASSWORD")
os.Unsetenv("WORKLOAD_MANAGER_ADDR")
os.Unsetenv("WORKLOAD_MANAGER_URL")
}()

config := &Config{
Expand Down
6 changes: 3 additions & 3 deletions pkg/router/session_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ type manager struct {

// NewSessionManager returns a SessionManager implementation.
// storeClient is used to query sandbox information from store
// workloadMgrAddr is read from the environment variable WORKLOAD_MANAGER_ADDR.
// workloadMgrAddr is read from the environment variable WORKLOAD_MANAGER_URL.
func NewSessionManager(storeClient store.Store) (SessionManager, error) {
workloadMgrAddr := os.Getenv("WORKLOAD_MANAGER_ADDR")
workloadMgrAddr := os.Getenv("WORKLOAD_MANAGER_URL")
if workloadMgrAddr == "" {
return nil, fmt.Errorf("WORKLOAD_MANAGER_ADDR environment variable is not set")
return nil, fmt.Errorf("WORKLOAD_MANAGER_URL environment variable is not set")
}

// Create HTTP transport with HTTP/2 support
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ type testEnv struct {
func newTestEnv(t *testing.T) *testEnv {
return &testEnv{
routerURL: getEnv("ROUTER_URL", defaultRouterURL),
workloadMgrURL: getEnv("WORKLOAD_MANAGER_ADDR", defaultWorkloadMgrURL),
workloadMgrURL: getEnv("WORKLOAD_MANAGER_URL", defaultWorkloadMgrURL),
authToken: os.Getenv("API_TOKEN"),
t: t,
}
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/run_e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,11 @@ collect_pod_logs() {
local label_selector=$1
local component_name=$2
local artifacts_dir=$3

echo "Collecting ${component_name} logs..."
local pods=$(kubectl -n "${AGENTCUBE_NAMESPACE}" get pods -l "${label_selector}" \
-o jsonpath='{.items[*].metadata.name}' 2>/dev/null || echo "")

if [ -n "$pods" ]; then
for pod in $pods; do
echo " Collecting logs from pod: $pod"
Expand Down Expand Up @@ -427,14 +427,14 @@ require_python
TEST_FAILED=0

echo "Running Go tests..."
if ! WORKLOAD_MANAGER_ADDR="http://localhost:${WORKLOAD_MANAGER_LOCAL_PORT}" ROUTER_URL="http://localhost:${ROUTER_LOCAL_PORT}" API_TOKEN=$API_TOKEN go test -v ./test/e2e/...; then
if ! WORKLOAD_MANAGER_URL="http://localhost:${WORKLOAD_MANAGER_LOCAL_PORT}" ROUTER_URL="http://localhost:${ROUTER_LOCAL_PORT}" API_TOKEN=$API_TOKEN go test -v ./test/e2e/...; then
TEST_FAILED=1
fi

echo "Running Python CodeInterpreter tests..."
cd "$(dirname "$0")"

if ! WORKLOAD_MANAGER_ADDR="http://localhost:${WORKLOAD_MANAGER_LOCAL_PORT}" ROUTER_URL="http://localhost:${ROUTER_LOCAL_PORT}" API_TOKEN=$API_TOKEN AGENTCUBE_NAMESPACE="${AGENTCUBE_NAMESPACE}" "$E2E_VENV_DIR/bin/python" test_codeinterpreter.py; then
if ! WORKLOAD_MANAGER_URL="http://localhost:${WORKLOAD_MANAGER_LOCAL_PORT}" ROUTER_URL="http://localhost:${ROUTER_LOCAL_PORT}" API_TOKEN=$API_TOKEN AGENTCUBE_NAMESPACE="${AGENTCUBE_NAMESPACE}" "$E2E_VENV_DIR/bin/python" test_codeinterpreter.py; then
TEST_FAILED=1
fi

Expand Down
30 changes: 18 additions & 12 deletions test/e2e/test_codeinterpreter.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,28 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import json
import os
import sys
import json
import unittest

# Import agentcube package (Installed in the virtual environment by run_e2e.sh)
from agentcube import CodeInterpreterClient
from agentcube.exceptions import CommandExecutionError


class TestCodeInterpreterE2E(unittest.TestCase):
"""E2E tests for CodeInterpreter functionality using Python SDK."""

def setUp(self):
"""Set up test environment."""
self.namespace = os.getenv("AGENTCUBE_NAMESPACE", "agentcube")
self.workload_manager_url = os.getenv("WORKLOAD_MANAGER_ADDR")
self.workload_manager_url = os.getenv("WORKLOAD_MANAGER_URL")
self.router_url = os.getenv("ROUTER_URL")
self.api_token = os.getenv("API_TOKEN")

if not self.workload_manager_url:
self.fail("WORKLOAD_MANAGER_ADDR environment variable not set")
self.fail("WORKLOAD_MANAGER_URL environment variable not set")

if not self.router_url:
self.fail("ROUTER_URL environment variable not set")
Expand Down Expand Up @@ -65,7 +66,7 @@ def test_case1_simple_code_execution_auto_session(self):
workload_manager_url=self.workload_manager_url,
router_url=self.router_url,
auth_token=self.api_token,
verbose=True
verbose=True,
) as client:
print(f"Session created: {client.session_id}")
self.assertIsNotNone(client.session_id, "Session ID should be created")
Expand Down Expand Up @@ -124,7 +125,7 @@ def test_case3_file_based_workflow_fibonacci_json(self):
"""
try:
# Create fibonacci.py script content
fibonacci_script = '''
fibonacci_script = """
import json

def fibonacci(n):
Expand All @@ -144,7 +145,7 @@ def fibonacci(n):
json.dump(result, f, indent=2)

print("Fibonacci sequence generated and saved to output.json")
'''
"""

# Expected result
expected_fib = [0, 1, 1, 2, 3, 5, 8, 13, 21, 34]
Expand All @@ -155,7 +156,7 @@ def fibonacci(n):
workload_manager_url=self.workload_manager_url,
router_url=self.router_url,
auth_token=self.api_token,
verbose=True
verbose=True,
) as client:
print(f"Session created: {client.session_id}")
self.assertIsNotNone(client.session_id, "Session ID should be created")
Expand All @@ -168,8 +169,9 @@ def fibonacci(n):
print("Executing fibonacci.py script...")
exec_result = client.run_code("python", fibonacci_script)
print(f"Script execution result: {repr(exec_result)}")
self.assertIn("Fibonacci sequence generated", exec_result,
f"Expected success message, got: {exec_result}")
self.assertIn(
"Fibonacci sequence generated", exec_result, f"Expected success message, got: {exec_result}"
)

# Step 3: Download and verify the output.json
print("Downloading output.json...")
Expand All @@ -185,12 +187,15 @@ def fibonacci(n):
self.assertEqual(
result_data["fibonacci_sequence"],
expected_fib,
f"Fibonacci mismatch: expected {expected_fib}, got {result_data['fibonacci_sequence']}"
f"Fibonacci mismatch: expected {expected_fib}, got {result_data['fibonacci_sequence']}",
)

# Assert: Check length
self.assertEqual(result_data["length"], len(expected_fib),
f"Length mismatch: expected {len(expected_fib)}, got {result_data['length']}")
self.assertEqual(
result_data["length"],
len(expected_fib),
f"Length mismatch: expected {len(expected_fib)}, got {result_data['length']}",
)
finally:
# Clean up temporary files
try:
Expand All @@ -200,6 +205,7 @@ def fibonacci(n):
except Exception as cleanup_error:
print(f"Warning: Failed to clean up temporary file: {cleanup_error}")


if __name__ == "__main__":
print("Starting CodeInterpreter E2E Tests...")
print(f"Python path: {sys.path}")
Expand Down
Loading