Skip to content
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

Adding public API test coverage Aspire.Hosting.NodeJs #5161

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/Aspire.Hosting.NodeJs/NodeAppResource.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using Aspire.Hosting.ApplicationModel;

namespace Aspire.Hosting;
Expand All @@ -11,7 +13,8 @@ namespace Aspire.Hosting;
/// <param name="command">The command to execute.</param>
/// <param name="workingDirectory">The working directory to use for the command. If null, the working directory of the current process is used.</param>
public class NodeAppResource(string name, string command, string workingDirectory)
: ExecutableResource(name, command, workingDirectory), IResourceWithServiceDiscovery
: ExecutableResource(ThrowIfNull(name), ThrowIfNull(command), ThrowIfNull(workingDirectory)), IResourceWithServiceDiscovery
{

private static string ThrowIfNull([NotNull] string? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null)
=> argument ?? throw new ArgumentNullException(paramName);
}
10 changes: 10 additions & 0 deletions src/Aspire.Hosting.NodeJs/NodeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ public static class NodeAppHostingExtension
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
public static IResourceBuilder<NodeAppResource> AddNodeApp(this IDistributedApplicationBuilder builder, string name, string scriptPath, string? workingDirectory = null, string[]? args = null)
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(name);
ArgumentNullException.ThrowIfNull(scriptPath);

args ??= [];
string[] effectiveArgs = [scriptPath, .. args];
workingDirectory ??= Path.GetDirectoryName(scriptPath)!;
Expand All @@ -45,6 +49,12 @@ public static IResourceBuilder<NodeAppResource> AddNodeApp(this IDistributedAppl
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
public static IResourceBuilder<NodeAppResource> AddNpmApp(this IDistributedApplicationBuilder builder, string name, string workingDirectory, string scriptName = "start", string[]? args = null)
{

ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(name);
ArgumentNullException.ThrowIfNull(workingDirectory);
ArgumentNullException.ThrowIfNull(scriptName);

string[] allArgs = args is { Length: > 0 }
? ["run", scriptName, "--", .. args]
: ["run", scriptName];
Expand Down
140 changes: 140 additions & 0 deletions tests/Aspire.Hosting.NodeJs.Tests/NodeJsPublicApiTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Xunit;

namespace Aspire.Hosting.NodeJs.Tests;

public class NodeJsPublicApiTests
{
[Fact]
public void AddNodeAppShouldThrowWhenBuilderIsNull()
{
IDistributedApplicationBuilder builder = null!;
var name = "nodeapp";
var scriptPath = ".\\app.js";

var action = () => builder.AddNodeApp(name: name, scriptPath: scriptPath);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(builder), exception.ParamName);
}

[Fact]
public void AddNodeAppShouldThrowWhenNameIsNull()
{
var builder = DistributedApplication.CreateBuilder();
string name = null!;
var scriptPath = ".\\app.js";

var action = () => builder.AddNodeApp(name: name, scriptPath: scriptPath);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(name), exception.ParamName);
}

[Fact]
public void AddNodeAppShouldThrowWhenScriptPathIsNull()
{
var builder = DistributedApplication.CreateBuilder();
var name = "nodeapp";
string scriptPath = null!;

var action = () => builder.AddNodeApp(name: name, scriptPath: scriptPath);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(scriptPath), exception.ParamName);
}

[Fact]
public void AddNpmAppShouldThrowWhenBuilderIsNull()
{
IDistributedApplicationBuilder builder = null!;
var name = "npmapp";
var workingDirectory = ".\\app";

var action = () => builder.AddNpmApp(name: name, workingDirectory: workingDirectory);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(builder), exception.ParamName);
}

[Fact]
public void AddNpmAppShouldThrowWhenWorkingDirectoryIsNull()
{
var builder = DistributedApplication.CreateBuilder();
var name = "npmapp";
string workingDirectory = null!;

var action = () => builder.AddNpmApp(name: name, workingDirectory: workingDirectory);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(workingDirectory), exception.ParamName);
}

[Fact]
public void AddNpmAppShouldThrowWhenNameIsNull()
{
var builder = DistributedApplication.CreateBuilder();
string name = null!;
var workingDirectory = ".\\app";

var action = () => builder.AddNpmApp(name: name, workingDirectory: workingDirectory);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(name), exception.ParamName);
}

[Fact]
public void AddNpmAppShouldThrowWhenScriptNameIsNull()
Copy link
Member

Choose a reason for hiding this comment

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

@Alirexaa you missed this one. It has a default value but it doesn't mean we still can't call it with a null value.

Copy link
Member

Choose a reason for hiding this comment

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

Also I had to force push to revert the changes in the .sln file.

{
var builder = DistributedApplication.CreateBuilder();
var name = "npmapp";
var workingDirectory = ".\\app";
string scriptName = null!;

var action = () => builder.AddNpmApp(name: name, workingDirectory: workingDirectory, scriptName: scriptName);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(scriptName), exception.ParamName);
}

[Fact]
public void CtorNodeAppResourceShouldThrowWhenNameIsNull()
{
string name = null!;
var command = "start";
var workingDirectory = ".\\app";

var action = () => new NodeAppResource(name, command, workingDirectory);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(name), exception.ParamName);
}

[Fact]
public void CtorNodeAppResourceShouldThrowWhenCommandIsNull()
{
var name = "nodeapp";
string command = null!;
var workingDirectory = ".\\app";

var action = () => new NodeAppResource(name, command, workingDirectory);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(command), exception.ParamName);
}

[Fact]
public void CtorNodeAppResourceShouldThrowWhenWorkingDirectoryIsNull()
{
var name = "nodeapp";
var command = "start";
string workingDirectory = null!;

var action = () => new NodeAppResource(name, command, workingDirectory);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal(nameof(workingDirectory), exception.ParamName);
}
}