Skip to content

Commit e982f47

Browse files
radicalCopilot
andcommitted
Fix review issues: disposed object return, unused field, resource leak, README
- Remove 'using' from CreateCommandWithMockGhAsync helpers that returned disposed ScriptToolCommand objects (PRScriptShellTests, PRScriptPowerShellTests) - Remove unused _forceShowBuildOutput field and parameter from ScriptToolCommand (would cause CS0414 with TreatWarningsAsErrors) - Add 'using' to HttpResponseMessage in ScriptHostFixture.InitializeAsync - Fix README: integration tests use [OuterloopTest] not [ActiveIssue] - Remove unrelated CI trigger pattern addition (create-failing-test-issue.*) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e75e8d9 commit e982f47

7 files changed

Lines changed: 9 additions & 14 deletions

File tree

eng/testing/github-ci-trigger-patterns.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ Aspire-Core.slnf
3939
.github/workflows/apply-test-attributes.yml
4040
.github/workflows/backmerge-release.yml
4141
.github/workflows/backport.yml
42-
.github/workflows/create-failing-test-issue.*
4342
.github/workflows/dogfood-comment.yml
4443
.github/workflows/generate-api-diffs.yml
4544
.github/workflows/generate-ats-diffs.yml

tests/Aspire.Acquisition.Tests/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,6 @@ GH_TOKEN=<token> dotnet test tests/Aspire.Acquisition.Tests -- --filter-trait "C
3636

3737
Integration tests (in `PRScriptIntegrationTests.cs`) query real GitHub PRs and are:
3838

39-
- Disabled in CI via `[ActiveIssue]`
39+
- Excluded from default CI runs via `[OuterloopTest]` and `Category=integration` trait filters
4040
- Require `GH_TOKEN` environment variable
4141
- Skip gracefully if no suitable PR is found

tests/Aspire.Acquisition.Tests/Scripts/Common/ScriptHostFixture.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public async ValueTask InitializeAsync()
5858

5959
// Verify the server is reachable
6060
using var client = new HttpClient();
61-
var response = await client.GetAsync($"{BaseUrl}/get-aspire-cli.sh");
61+
using var response = await client.GetAsync($"{BaseUrl}/get-aspire-cli.sh");
6262
if (!response.IsSuccessStatusCode)
6363
{
6464
throw new InvalidOperationException($"Script host failed to start: HTTP {response.StatusCode}");

tests/Aspire.Acquisition.Tests/Scripts/Common/ScriptToolCommand.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,21 @@ public class ScriptToolCommand : ToolCommand
1414
{
1515
private readonly string _scriptPath;
1616
private readonly TestEnvironment _testEnvironment;
17-
private readonly bool _forceShowBuildOutput;
1817

1918
/// <summary>
2019
/// Creates a new command to execute a script.
2120
/// </summary>
2221
/// <param name="scriptPath">Relative path to the script from repo root (e.g., "eng/scripts/get-aspire-cli.sh")</param>
2322
/// <param name="testEnvironment">Test environment providing isolated temp directories</param>
2423
/// <param name="testOutput">xUnit test output helper</param>
25-
/// <param name="forceShowBuildOutput">Whether to force showing build output in the test results</param>
2624
public ScriptToolCommand(
2725
string scriptPath,
2826
TestEnvironment testEnvironment,
29-
ITestOutputHelper testOutput,
30-
bool forceShowBuildOutput = false)
27+
ITestOutputHelper testOutput)
3128
: base(GetExecutable(scriptPath), testOutput, label: Path.GetFileName(scriptPath))
3229
{
3330
_scriptPath = scriptPath;
3431
_testEnvironment = testEnvironment;
35-
_forceShowBuildOutput = forceShowBuildOutput;
3632

3733
// Set mock HOME to prevent any accidental user directory access
3834
WithEnvironmentVariable("HOME", _testEnvironment.MockHome);

tests/Aspire.Acquisition.Tests/Scripts/PRScriptIntegrationTests.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public async Task ShellScript_WithRealPR_DryRun_Succeeds()
3636
}
3737

3838
using var env = new TestEnvironment();
39-
using var cmd = new ScriptToolCommand(ScriptPaths.PRShell, env, _testOutput, forceShowBuildOutput: true);
39+
using var cmd = new ScriptToolCommand(ScriptPaths.PRShell, env, _testOutput);
4040

4141
var result = await cmd.ExecuteAsync(
4242
_prFixture.PRNumber.ToString(),
@@ -57,7 +57,7 @@ public async Task PowerShellScript_WithRealPR_WhatIf_Succeeds()
5757
}
5858

5959
using var env = new TestEnvironment();
60-
using var cmd = new ScriptToolCommand(ScriptPaths.PRPowerShell, env, _testOutput, forceShowBuildOutput: true);
60+
using var cmd = new ScriptToolCommand(ScriptPaths.PRPowerShell, env, _testOutput);
6161

6262
var result = await cmd.ExecuteAsync(
6363
"-PRNumber", _prFixture.PRNumber.ToString(),
@@ -78,7 +78,7 @@ public async Task ShellScript_WithRealPR_DiscoverRunId_DryRun_Succeeds()
7878
}
7979

8080
using var env = new TestEnvironment();
81-
using var cmd = new ScriptToolCommand(ScriptPaths.PRShell, env, _testOutput, forceShowBuildOutput: true);
81+
using var cmd = new ScriptToolCommand(ScriptPaths.PRShell, env, _testOutput);
8282

8383
// Test automatic run ID discovery by only passing PR number
8484
var result = await cmd.ExecuteAsync(
@@ -99,7 +99,7 @@ public async Task PowerShellScript_WithRealPR_DiscoverRunId_WhatIf_Succeeds()
9999
}
100100

101101
using var env = new TestEnvironment();
102-
using var cmd = new ScriptToolCommand(ScriptPaths.PRPowerShell, env, _testOutput, forceShowBuildOutput: true);
102+
using var cmd = new ScriptToolCommand(ScriptPaths.PRPowerShell, env, _testOutput);
103103

104104
// Test automatic run ID discovery by only passing PR number
105105
var result = await cmd.ExecuteAsync(

tests/Aspire.Acquisition.Tests/Scripts/PRScriptPowerShellTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public class PRScriptPowerShellTests(ITestOutputHelper testOutput)
2222
private async Task<ScriptToolCommand> CreateCommandWithMockGhAsync(TestEnvironment env)
2323
{
2424
var mockGhPath = await env.CreateMockGhScriptAsync(_testOutput);
25-
using var cmd = new ScriptToolCommand(s_scriptPath, env, _testOutput);
25+
var cmd = new ScriptToolCommand(s_scriptPath, env, _testOutput);
2626
cmd.WithEnvironmentVariable("PATH", $"{mockGhPath}{Path.PathSeparator}{Environment.GetEnvironmentVariable("PATH")}");
2727
return cmd;
2828
}

tests/Aspire.Acquisition.Tests/Scripts/PRScriptShellTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public class PRScriptShellTests(ITestOutputHelper testOutput)
1919
private async Task<ScriptToolCommand> CreateCommandWithMockGhAsync(TestEnvironment env)
2020
{
2121
var mockGhPath = await env.CreateMockGhScriptAsync(_testOutput);
22-
using var cmd = new ScriptToolCommand(s_scriptPath, env, _testOutput);
22+
var cmd = new ScriptToolCommand(s_scriptPath, env, _testOutput);
2323
cmd.WithEnvironmentVariable("PATH", $"{mockGhPath}{Path.PathSeparator}{Environment.GetEnvironmentVariable("PATH")}");
2424
return cmd;
2525
}

0 commit comments

Comments
 (0)