Skip to content

Adding public API test coverage for Aspire.Hosting.Python #5110

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

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c67cdf3
CA1062#Aspire.Hosting.Python
Zombach Jul 29, 2024
ad059e5
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Jul 29, 2024
f3bd0d3
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Jul 29, 2024
ec25311
Fix PR by feedback
Zombach Jul 30, 2024
96efb2e
Merge branch 'zombach/validate-arguments-of-public-methods#aspire-hos…
Zombach Jul 30, 2024
90a67ac
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Jul 30, 2024
386bd9c
Remove Assert.Multiple by feedback
Zombach Jul 30, 2024
baf09c4
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Jul 31, 2024
d038841
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Jul 31, 2024
0ad4c44
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 1, 2024
fc8f9b2
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 1, 2024
f75eb99
builder replace to TestDistributedApplicationBuilder
Zombach Aug 1, 2024
1179728
To expression body
Zombach Aug 1, 2024
1961d80
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 2, 2024
a21b603
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 4, 2024
9fd8ab6
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 5, 2024
4466e5d
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 6, 2024
a54821a
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 8, 2024
be2c5c8
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 14, 2024
3616f3a
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 17, 2024
d2cd1b4
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 20, 2024
2420a46
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 20, 2024
0fa62c5
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 24, 2024
ae319dc
Merge branch 'dotnet:main' into zombach/validate-arguments-of-public-…
Zombach Aug 24, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public static class PythonProjectResourceBuilderExtensions
public static IResourceBuilder<PythonProjectResource> AddPythonProject(
this IDistributedApplicationBuilder builder, string name, string projectDirectory, string scriptPath, params string[] scriptArgs)
{
ArgumentNullException.ThrowIfNull(builder);

return builder.AddPythonProject(name, projectDirectory, scriptPath, ".venv", scriptArgs);
}

Expand Down Expand Up @@ -108,7 +110,12 @@ public static IResourceBuilder<PythonProjectResource> AddPythonProject(
this IDistributedApplicationBuilder builder, string name, string projectDirectory, string scriptPath,
string virtualEnvironmentPath, params string[] scriptArgs)
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(name);
ArgumentNullException.ThrowIfNull(projectDirectory);
ArgumentNullException.ThrowIfNull(scriptPath);
ArgumentNullException.ThrowIfNull(virtualEnvironmentPath);
ArgumentNullException.ThrowIfNull(scriptArgs);

projectDirectory = PathNormalizer.NormalizePathForCurrentPlatform(Path.Combine(builder.AppHostDirectory, projectDirectory));
var virtualEnvironment = new VirtualEnvironment(Path.IsPathRooted(virtualEnvironmentPath)
Expand Down
277 changes: 277 additions & 0 deletions tests/Aspire.Hosting.Python.Tests/PythonPublicApiTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,277 @@
// 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.Python.Tests;

public class PythonPublicApiTests
{
#region PythonProjectResource

[Fact]
public void CtorPythonProjectResourceShouldThrowWhenNameIsNull()
{
string name = null!;
const string executablePath = "/src/python";
const string projectDirectory = "/data/python";

var action = () => new PythonProjectResource(name, executablePath, projectDirectory);

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

[Fact]
public void CtorPythonProjectResourceShouldThrowWhenExecutablePathIsNull()
{
const string name = "Python";
string executablePath = null!;
const string projectDirectory = "/data/python";

var action = () => new PythonProjectResource(name, executablePath, projectDirectory);

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

[Fact]
public void CtorPythonProjectResourceShouldThrowWhenProjectDirectoryIsNull()
{
const string name = "Python";
const string executablePath = "/src/python";
string projectDirectory = null!;

var action = () => new PythonProjectResource(name, executablePath, projectDirectory);

var exception = Assert.Throws<ArgumentNullException>(action);
Assert.Equal("workingDirectory", exception.ParamName);
Copy link
Member

Choose a reason for hiding this comment

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

PythonProjectResource's parameter is named projectDirectory which is then passed to the ExecutableResource..ctor to the parameter named workingDirectory.

And because the exception is being thrown from ExecutableResource we get workingDirectory instead of projectDirectory. That sounds confusing for the user. Maybe we should have the null check on the .ctor arguments for PythonProjectResource directly.

@mitchdenny @eerhardt @sebastienros Is there a pattern that we follow in such cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I would add a check, it would be more transparent.
But due to the fact that in PythonProjectResource these arguments are not used at the moment. according to CA1062, we can skip this check, since it is performed in depth.
But if someone adds interaction with arguments to PythonProjectResource, then they will have to add a check for 'null'.
That's why I'm in favor of adding it right away.

Copy link
Member

Choose a reason for hiding this comment

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

https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/using-standard-exception-types

✔️ DO set the ParamName property when throwing one of the subclasses of ArgumentException.

This property represents the name of the parameter that caused the exception to be thrown.

I read this as the ParamName should be set to the name of the parameter that was invalid. In this case projectDirectory.

Copy link
Member

Choose a reason for hiding this comment

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

I think we need to do the null check in the PythonResource ctor just for clarity. I'm not a fan of the ThrowIfNull method, seems a bit odd to do it that way.

Copy link
Member

Choose a reason for hiding this comment

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

On second thought it probably doesn't matter to the end result.

}

#endregion

#region PythonProjectResourceBuilderExtensions

[Fact]
public void AddPythonProjectShouldThrowWhenBuilderIsNull()
{
IDistributedApplicationBuilder builder = null!;
const string name = "Python";
const string projectDirectory = "/src/python";
const string scriptPath = "scripts";
string[] scriptArgs = ["--traces"];

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectShouldThrowWhenNameIsNull()
{
var builder = DistributedApplication.CreateBuilder();
string name = null!;
const string projectDirectory = "/src/python";
const string scriptPath = "scripts";
string[] scriptArgs = ["--traces"];

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectShouldThrowWhenProjectDirectoryIsNull()
{
var builder = DistributedApplication.CreateBuilder();
const string name = "Python";
string projectDirectory = null!;
const string scriptPath = "scripts";
string[] scriptArgs = ["--traces"];

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectShouldThrowWhenScriptPathIsNull()
{
var builder = DistributedApplication.CreateBuilder();
const string name = "Python";
const string projectDirectory = "/src/python";
string scriptPath = null!;
string[] scriptArgs = ["--traces"];

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectShouldThrowWhenScriptArgsIsNull()
{
var builder = DistributedApplication.CreateBuilder();
const string name = "Python";
const string projectDirectory = "/src/python";
const string scriptPath = "scripts";
string[] scriptArgs = null!;

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectWithVirtualEnvironmentPathShouldThrowWhenBuilderIsNull()
{
IDistributedApplicationBuilder builder = null!;
const string name = "Python";
const string projectDirectory = "/src/python";
const string scriptPath = "scripts";
var virtualEnvironmentPath = ".venv";
string[] scriptArgs = ["--traces"]; ;

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
virtualEnvironmentPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectWithVirtualEnvironmentPathShouldThrowWhenNameIsNull()
{
var builder = DistributedApplication.CreateBuilder();
string name = null!;
const string projectDirectory = "/src/python";
const string scriptPath = "scripts";
const string virtualEnvironmentPath = ".venv";
string[] scriptArgs = ["--traces"]; ;

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
virtualEnvironmentPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectWithVirtualEnvironmentPathShouldThrowWhenProjectDirectoryIsNull()
{
var builder = DistributedApplication.CreateBuilder();
const string name = "Python";
string projectDirectory = null!;
const string scriptPath = "scripts";
const string virtualEnvironmentPath = ".venv";
string[] scriptArgs = ["--traces"]; ;

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
virtualEnvironmentPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectWithVirtualEnvironmentPathShouldThrowWhenScriptPathIsNull()
{
var builder = DistributedApplication.CreateBuilder();
const string name = "Python";
const string projectDirectory = "/src/python";
string scriptPath = null!;
const string virtualEnvironmentPath = ".venv";
string[] scriptArgs = ["--traces"]; ;

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
virtualEnvironmentPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectWithVirtualEnvironmentPathShouldThrowWhenVirtualEnvironmentPathIsNull()
{
var builder = DistributedApplication.CreateBuilder();
const string name = "Python";
const string projectDirectory = "/src/python";
const string scriptPath = "scripts";
string virtualEnvironmentPath = null!;
string[] scriptArgs = ["--traces"]; ;

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
virtualEnvironmentPath,
scriptArgs);

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

[Fact]
public void AddPythonProjectWithVirtualEnvironmentPathShouldThrowWhenScriptArgsIsNull()
{
var builder = DistributedApplication.CreateBuilder();
const string name = "Python";
const string projectDirectory = "/src/python";
const string scriptPath = "scripts";
const string virtualEnvironmentPath = ".venv";
string[] scriptArgs = null!;

var action = () => builder.AddPythonProject(
name,
projectDirectory,
scriptPath,
virtualEnvironmentPath,
scriptArgs);

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

#endregion
}
Loading