Skip to content

Commit 0f9f65e

Browse files
authored
Do not write to stderr when checking if an exe has a manifest. (#207)
Fixes #205.
1 parent e773d6a commit 0f9f65e

File tree

5 files changed

+80
-15
lines changed

5 files changed

+80
-15
lines changed

src/winapp-CLI/WinApp.Cli.Tests/BaseCommandTests.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
namespace WinApp.Cli.Tests;
1010

11-
public abstract class BaseCommandTests(bool configPaths = true)
11+
public abstract class BaseCommandTests(bool configPaths = true, bool verboseLogging = true)
1212
{
1313
private protected DirectoryInfo _tempDirectory = null!;
1414
private protected DirectoryInfo _testWinappDirectory = null!;
@@ -46,7 +46,8 @@ public void SetupBase()
4646
{
4747
b.ClearProviders();
4848
b.AddTextWriterLogger([Console.Out, ConsoleStdOut], [Console.Error, ConsoleStdErr]);
49-
b.SetMinimumLevel(LogLevel.Debug);
49+
// Use Debug level for verbose logging, Information level for non-verbose
50+
b.SetMinimumLevel(verboseLogging ? LogLevel.Debug : LogLevel.Information);
5051
});
5152

5253
_serviceProvider = services.BuildServiceProvider();

src/winapp-CLI/WinApp.Cli.Tests/BuildToolsServiceTests.cs

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
using WinApp.Cli.Services;
45
using WinApp.Cli.Tools;
56

67
namespace WinApp.Cli.Tests;
@@ -159,7 +160,7 @@ public async Task RunBuildToolAsync_WithValidTool_ReturnsOutput()
159160
File.WriteAllText(fakeToolPath, "@echo Hello from fake tool");
160161

161162
// Act
162-
var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync(new GenericTool("echo.cmd"), "", TestContext.CancellationToken);
163+
var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync(new GenericTool("echo.cmd"), "", true, TestContext.CancellationToken);
163164

164165
// Assert
165166
Assert.Contains("Hello from fake tool", stdout);
@@ -175,7 +176,7 @@ public async Task RunBuildToolAsync_WithNonExistentTool_ThrowsFileNotFoundExcept
175176
// Act & Assert
176177
await Assert.ThrowsExactlyAsync<FileNotFoundException>(async () =>
177178
{
178-
await _buildToolsService.RunBuildToolAsync(new GenericTool("nonexistent.exe"), "", TestContext.CancellationToken);
179+
await _buildToolsService.RunBuildToolAsync(new GenericTool("nonexistent.exe"), "", true, TestContext.CancellationToken);
179180
});
180181
}
181182

@@ -322,7 +323,7 @@ public async Task RunBuildToolAsync_WithNoExistingPackage_AutoInstallsAndRuns()
322323
{
323324
// Create a simple batch command that outputs something
324325
// This will either succeed (if BuildTools installs successfully) or throw an exception
325-
await _buildToolsService.RunBuildToolAsync(new GenericTool("echo.cmd"), "test", TestContext.CancellationToken);
326+
await _buildToolsService.RunBuildToolAsync(new GenericTool("echo.cmd"), "test", true, TestContext.CancellationToken);
326327

327328
// If we reach here, the auto-installation worked - test passes
328329
}
@@ -349,10 +350,71 @@ public async Task RunBuildToolAsync_WithExistingTool_RunsDirectly()
349350
File.WriteAllText(batchFile, "@echo Hello from test tool");
350351

351352
// Act
352-
var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync(new GenericTool("test.cmd"), "", TestContext.CancellationToken);
353+
var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync(new GenericTool("test.cmd"), "", true, TestContext.CancellationToken);
353354

354355
// Assert
355356
Assert.Contains("Hello from test tool", stdout);
356357
Assert.AreEqual(string.Empty, stderr.Trim());
357358
}
358359
}
360+
361+
/// <summary>
362+
/// Tests for BuildToolsService with non-verbose logging to test PrintErrorText behavior
363+
/// </summary>
364+
[TestClass]
365+
[DoNotParallelize]
366+
public class BuildToolsServicePrintErrorsTests() : BaseCommandTests(configPaths: true, verboseLogging: false)
367+
{
368+
[TestMethod]
369+
public async Task RunBuildToolAsync_WithPrintErrorsTrue_WritesErrorOutput()
370+
{
371+
// Arrange - Create a batch file that fails with exit code 1 and writes to stderr
372+
var packagesDir = Path.Combine(_testCacheDirectory.FullName, "packages");
373+
var buildToolsPackageDir = Path.Combine(packagesDir, "Microsoft.Windows.SDK.BuildTools.10.0.26100.1");
374+
var binDir = Path.Combine(buildToolsPackageDir, "bin", "10.0.26100.0", "x64");
375+
Directory.CreateDirectory(binDir);
376+
377+
var failingTool = Path.Combine(binDir, "failing.cmd");
378+
File.WriteAllText(failingTool, "@echo Error message to stderr 1>&2\r\n@exit /b 1");
379+
380+
// Act: Run with printErrors=true - error SHOULD be printed via PrintErrorText
381+
var exception = await Assert.ThrowsExactlyAsync<BuildToolsService.InvalidBuildToolException>(async () =>
382+
{
383+
await _buildToolsService.RunBuildToolAsync(new GenericTool("failing.cmd"), "", printErrors: true, TestContext.CancellationToken);
384+
});
385+
386+
// Verify the exception captures the error info
387+
Assert.Contains("Error message to stderr", exception.Stderr);
388+
389+
// Verify that error-level log output occurred when printErrors=true
390+
// (PrintErrorText uses LogError which goes to stderr)
391+
var stdErrOutput = ConsoleStdErr.ToString();
392+
Assert.Contains("Error message to stderr", stdErrOutput, "Error output should be printed when printErrors is true");
393+
}
394+
395+
[TestMethod]
396+
public async Task RunBuildToolAsync_WithPrintErrorsFalse_DoesNotWriteErrorOutput()
397+
{
398+
// Arrange - Create a batch file that fails with exit code 1 and writes to stderr
399+
var packagesDir = Path.Combine(_testCacheDirectory.FullName, "packages");
400+
var buildToolsPackageDir = Path.Combine(packagesDir, "Microsoft.Windows.SDK.BuildTools.10.0.26100.1");
401+
var binDir = Path.Combine(buildToolsPackageDir, "bin", "10.0.26100.0", "x64");
402+
Directory.CreateDirectory(binDir);
403+
404+
var failingTool = Path.Combine(binDir, "failing.cmd");
405+
File.WriteAllText(failingTool, "@echo Error message to stderr 1>&2\r\n@exit /b 1");
406+
407+
// Act: Run with printErrors=false - error should NOT be printed via PrintErrorText
408+
var exception = await Assert.ThrowsExactlyAsync<BuildToolsService.InvalidBuildToolException>(async () =>
409+
{
410+
await _buildToolsService.RunBuildToolAsync(new GenericTool("failing.cmd"), "", printErrors: false, TestContext.CancellationToken);
411+
});
412+
413+
// Verify the exception still captures the error info
414+
Assert.Contains("Error message to stderr", exception.Stderr);
415+
416+
// Verify that NO error-level log output occurred when printErrors=false
417+
var stdErrOutput = ConsoleStdErr.ToString();
418+
Assert.DoesNotContain("Error message to stderr", stdErrOutput, "Error output should NOT be printed when printErrors is false");
419+
}
420+
}

src/winapp-CLI/WinApp.Cli/Services/BuildToolsService.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,10 @@ public async Task<FileInfo> EnsureBuildToolAvailableAsync(string toolName, Cance
288288
/// </summary>
289289
/// <param name="tool">The tool to execute</param>
290290
/// <param name="arguments">Arguments to pass to the tool</param>
291+
/// <param name="printErrors">Whether to print errors using the tool's PrintErrorText method</param>
291292
/// <param name="cancellationToken">Cancellation token</param>
292293
/// <returns>Tuple containing (stdout, stderr)</returns>
293-
public async Task<(string stdout, string stderr)> RunBuildToolAsync(Tool tool, string arguments, CancellationToken cancellationToken = default)
294+
public async Task<(string stdout, string stderr)> RunBuildToolAsync(Tool tool, string arguments, bool printErrors, CancellationToken cancellationToken = default)
294295
{
295296
cancellationToken.ThrowIfCancellationRequested();
296297

@@ -328,7 +329,7 @@ public async Task<FileInfo> EnsureBuildToolAvailableAsync(string toolName, Cance
328329
{
329330
// Print tool-specific error output when not in verbose mode
330331
// In verbose mode, all output is already visible via LogDebug above
331-
if (!logger.IsEnabled(LogLevel.Debug))
332+
if (!logger.IsEnabled(LogLevel.Debug) && printErrors)
332333
{
333334
tool.PrintErrorText(stdout, stderr, logger);
334335
}

src/winapp-CLI/WinApp.Cli/Services/IBuildToolsService.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,14 @@ internal interface IBuildToolsService
2828
Task<FileInfo> EnsureBuildToolAvailableAsync(string toolName, CancellationToken cancellationToken = default);
2929

3030
Task<DirectoryInfo?> EnsureBuildToolsAsync(bool forceLatest = false, CancellationToken cancellationToken = default);
31-
31+
3232
/// <summary>
3333
/// Execute a build tool with the specified arguments
3434
/// </summary>
3535
/// <param name="tool">The tool to execute</param>
3636
/// <param name="arguments">Arguments to pass to the tool</param>
37+
/// <param name="printErrors">Whether to print errors using the tool's PrintErrorText method</param>
3738
/// <param name="cancellationToken">Cancellation token</param>
3839
/// <returns>Tuple containing (stdout, stderr)</returns>
39-
Task<(string stdout, string stderr)> RunBuildToolAsync(Tool tool, string arguments, CancellationToken cancellationToken = default);
40+
Task<(string stdout, string stderr)> RunBuildToolAsync(Tool tool, string arguments, bool printErrors = true, CancellationToken cancellationToken = default);
4041
}

src/winapp-CLI/WinApp.Cli/Services/MsixService.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ private async Task EmbedManifestFileToExeAsync(
442442
logger.LogDebug("Merging with existing manifest using mt.exe...");
443443

444444
// Use mt.exe to merge existing manifest with new manifest
445-
await RunMtToolAsync($@"-manifest ""{tempManifestPath}"" ""{manifestPath}"" -out:""{mergedManifestPath}""", cancellationToken);
445+
await RunMtToolAsync($@"-manifest ""{tempManifestPath}"" ""{manifestPath}"" -out:""{mergedManifestPath}""", true, cancellationToken);
446446
}
447447
else
448448
{
@@ -455,7 +455,7 @@ private async Task EmbedManifestFileToExeAsync(
455455
logger.LogDebug("Embedding merged manifest into executable...");
456456

457457
// Update the executable with merged manifest
458-
await RunMtToolAsync($@"-manifest ""{mergedManifestPath}"" -outputresource:""{exePath}"";#1", cancellationToken);
458+
await RunMtToolAsync($@"-manifest ""{mergedManifestPath}"" -outputresource:""{exePath}"";#1", true, cancellationToken);
459459

460460
logger.LogDebug("{UISymbol} Successfully embedded manifest into: {ExecutablePath}", UiSymbols.Check, exePath);
461461
}
@@ -479,7 +479,7 @@ private async Task<bool> TryExtractManifestFromExeAsync(FileInfo exePath, FileIn
479479
bool hasExistingManifest = false;
480480
try
481481
{
482-
await RunMtToolAsync($@"-inputresource:""{exePath}"";#1 -out:""{tempManifestPath}""", cancellationToken);
482+
await RunMtToolAsync($@"-inputresource:""{exePath}"";#1 -out:""{tempManifestPath}""", false, cancellationToken);
483483
tempManifestPath.Refresh();
484484
hasExistingManifest = tempManifestPath.Exists;
485485
}
@@ -1133,10 +1133,10 @@ private async Task CreateMsixPackageFromFolderAsync(DirectoryInfo inputFolder, F
11331133
await buildToolsService.RunBuildToolAsync(new MakeAppxTool(), makeappxArguments, cancellationToken: cancellationToken);
11341134
}
11351135

1136-
private async Task RunMtToolAsync(string arguments, CancellationToken cancellationToken = default)
1136+
private async Task RunMtToolAsync(string arguments, bool printErrors, CancellationToken cancellationToken = default)
11371137
{
11381138
// Use BuildToolsService to run mt.exe
1139-
await buildToolsService.RunBuildToolAsync(new GenericTool("mt.exe"), arguments, cancellationToken: cancellationToken);
1139+
await buildToolsService.RunBuildToolAsync(new GenericTool("mt.exe"), arguments, printErrors, cancellationToken: cancellationToken);
11401140
}
11411141

11421142
private static void TryDeleteFile(FileInfo path)

0 commit comments

Comments
 (0)