-
Notifications
You must be signed in to change notification settings - Fork 64
chore(test): mock openshell in TestRunAgent_* to prevent hangs #2986
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -139,11 +139,23 @@ func TestRunCommand_RejectsNegativeMaxResources(t *testing.T) { | |
| assert.Contains(t, err.Error(), "--max-resources must be >= 1") | ||
| } | ||
|
|
||
| // useFakeOpenshell prepends testdata/ to PATH so the stub openshell binary | ||
| // is found instead of a real installation, causing tests to fail fast at | ||
| // sandbox.EnsureAvailable instead of actually running agents. | ||
| func useFakeOpenshell(t *testing.T) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] test-correctness Two comments incorrectly state the failure point. (1) The existing comment in TestRunAgent_HarnessLoadPipeline (line 145) says fails later at sandbox.EnsureAvailable (no openshell in test env) but with the fake stub in PATH, EnsureAvailable succeeds (it only calls exec.LookPath) and the actual failure occurs at sandbox.CheckGateway (line 564 of run.go), which invokes openshell gateway list and gets exit code 1 from the stub. (2) The new useFakeOpenshell doc comment also says causing tests to fail fast at sandbox.EnsureAvailable — this is equally wrong for the same reason. Suggested fix: Update both comments: (1) in TestRunAgent_HarnessLoadPipeline, change fails later at sandbox.EnsureAvailable (no openshell in test env) to fails later at sandbox.CheckGateway (stub exits 1); (2) in useFakeOpenshell, change causing tests to fail fast at sandbox.EnsureAvailable to causing tests to fail fast at sandbox.CheckGateway. |
||
| t.Helper() | ||
| testdataDir, err := filepath.Abs("testdata") | ||
| require.NoError(t, err) | ||
| origPath := os.Getenv("PATH") | ||
| t.Setenv("PATH", testdataDir+string(filepath.ListSeparator)+origPath) | ||
| } | ||
|
rh-hemartin marked this conversation as resolved.
|
||
|
|
||
| func TestRunAgent_HarnessLoadPipeline(t *testing.T) { | ||
| // Exercises the early runAgent pipeline: absFullsendDir, policy, | ||
| // org config loading, LoadWithBase, baseDeps, ResolveRelativeTo. | ||
| // The function fails later at sandbox.EnsureAvailable (no openshell | ||
| // in test env), but by then all harness-loading code paths are covered. | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) | ||
|
|
@@ -168,6 +180,7 @@ func TestRunAgent_HarnessLoadPipeline(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_YMLFallback(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) | ||
|
|
@@ -192,6 +205,7 @@ func TestRunAgent_YMLFallback(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_HarnessNotFound(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
|
|
||
|
|
@@ -206,6 +220,7 @@ func TestRunAgent_HarnessNotFound(t *testing.T) { | |
| func TestRunAgent_HarnessLoadWithOrgConfig(t *testing.T) { | ||
| // Same as above but with a config.yaml present, covering the | ||
| // orgCfg != nil → orgAllowlist path. | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) | ||
|
|
@@ -237,6 +252,7 @@ func TestRunAgent_HarnessLoadWithOrgConfig(t *testing.T) { | |
| func TestRunAgent_MalformedOrgConfig(t *testing.T) { | ||
| // A malformed config.yaml should produce a warning but not prevent | ||
| // local-only harnesses from proceeding through the pipeline. | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) | ||
|
|
@@ -266,6 +282,7 @@ func TestRunAgent_MalformedOrgConfig(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_MalformedOrgConfigWithURLRefs(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| // A malformed config.yaml with URL-referenced resources should fail | ||
| // with a parse error on the re-attempt inside HasURLReferences. | ||
| agentHash := fetch.ComputeSHA256([]byte("agent content")) | ||
|
|
@@ -292,6 +309,7 @@ func TestRunAgent_MalformedOrgConfigWithURLRefs(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_URLRefsNoOrgConfig(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| // Harness with URL agent but no config.yaml → exercises the | ||
| // orgCfg == nil path inside HasURLReferences. | ||
| dir := t.TempDir() | ||
|
|
@@ -314,6 +332,7 @@ func TestRunAgent_URLRefsNoOrgConfig(t *testing.T) { | |
|
|
||
| func TestRunAgent_WithURLBase(t *testing.T) { | ||
| // Harness with a URL base — exercises the baseDeps logging loop. | ||
| useFakeOpenshell(t) | ||
| baseContent := []byte("agent: agents/shared.md\nrole: test\n") | ||
| baseHash := fetch.ComputeSHA256(baseContent) | ||
|
|
||
|
|
@@ -354,6 +373,7 @@ func TestRunAgent_WithURLBase(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_URLBaseNoOrgConfig(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| // Harness with a URL base but no config.yaml — exercises the | ||
| // pre-check that loads config strictly when a URL base is detected. | ||
| baseContent := []byte("agent: agents/shared.md\n") | ||
|
|
@@ -379,6 +399,7 @@ func TestRunAgent_URLBaseNoOrgConfig(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_URLBaseMalformedOrgConfig(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| // Harness with a URL base and malformed config.yaml — exercises the | ||
| // pre-check parse error path. | ||
| baseContent := []byte("agent: agents/shared.md\n") | ||
|
|
@@ -479,6 +500,7 @@ func TestRunCommand_HasEnvFileFlag(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_ConfigAgentLocalPath(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) | ||
|
|
@@ -508,6 +530,7 @@ func TestRunAgent_ConfigAgentLocalPath(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_ConfigAgentURL(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| harnessContent := []byte("agent: agents/remote.md\nrole: test\n") | ||
| harnessHash := fetch.ComputeSHA256(harnessContent) | ||
|
|
||
|
|
@@ -543,6 +566,7 @@ func TestRunAgent_ConfigAgentURL(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_ConfigAgentOverridesScaffold(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| // When config has an agent with the same name as a scaffold agent, | ||
| // the config source is used instead of the scaffold wrapper. | ||
| dir := t.TempDir() | ||
|
|
@@ -576,6 +600,7 @@ func TestRunAgent_ConfigAgentOverridesScaffold(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_ScaffoldFallback(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| // When config has agents but the requested agent is not in config, | ||
| // fall back to disk-based resolution. | ||
| dir := t.TempDir() | ||
|
|
@@ -608,6 +633,7 @@ func TestRunAgent_ScaffoldFallback(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_UnknownAgentName(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
|
|
||
|
|
@@ -2336,6 +2362,7 @@ func TestEmitDiagnosticWithContext(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_ErrorOnMissingRole(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| // Verifies that runAgent fails with a hard error when harness has no role. | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
|
|
@@ -2822,6 +2849,7 @@ func TestMintAgentToken_CleanupRestoresOriginals(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_FallsBackToFULLSEND_MINT_URL(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) | ||
|
|
@@ -2865,6 +2893,7 @@ func TestRunAgent_FallsBackToFULLSEND_MINT_URL(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_WarnsWhenNoMintURL(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) | ||
|
|
@@ -2901,6 +2930,7 @@ func TestRunAgent_WarnsWhenNoMintURL(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_MintTokenError(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) | ||
|
|
@@ -2937,6 +2967,7 @@ func TestRunAgent_MintTokenError(t *testing.T) { | |
| } | ||
|
|
||
| func TestRunAgent_StatusNotifierSetup(t *testing.T) { | ||
| useFakeOpenshell(t) | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) | ||
| require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| #!/bin/sh | ||
| # Fake openshell that always fails — used by TestRunAgent_* tests to prevent | ||
| # real sandbox execution. Tests expect to fail at sandbox.EnsureAvailable. | ||
| echo "fake openshell stub for tests" >&2 | ||
| exit 1 |
Uh oh!
There was an error while loading. Please reload this page.