Skip to content
Merged
Copy link
Contributor

Choose a reason for hiding this comment

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

@ChrisDoernen Can you please restore the original implementation of ConfigurationTersts class and instead add a new configuration tests class to verify TestProjectFixtureWithoutAppsettings's implementation?

Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
namespace Xunit.Microsoft.DependencyInjection.ExampleTests;

public class ConfigurationTests : TestBed<TestProjectFixture>
public class ConfigurationTests : TestBed<TestProjectFixtureWithoutAppsettings>
{
private const string Key = "CONFIG_KEY";
private const string Value = "Value";

public ConfigurationTests(ITestOutputHelper testOutputHelper, TestProjectFixture fixture) : base(testOutputHelper, fixture)
public ConfigurationTests(ITestOutputHelper testOutputHelper, TestProjectFixtureWithoutAppsettings fixture)
: base(testOutputHelper, fixture)
{
Environment.SetEnvironmentVariable(Key, Value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

public class TestProjectFixture : TestBedFixture
{
protected override void AddServices(IServiceCollection services, IConfiguration? configuration)
protected override void AddServices(IServiceCollection services, IConfiguration configuration)
=> services
// Transient services - new instance for each injection
.AddTransient<ICalculator, Calculator>()
Expand All @@ -23,8 +23,8 @@ protected override void AddServices(IServiceCollection services, IConfiguration?
.AddTransient<Func<ISingletonService>>(provider => () => provider.GetService<ISingletonService>()!)

// Configure options
.Configure<Options>(config => configuration?.GetSection("Options").Bind(config))
.Configure<SecretValues>(config => configuration?.GetSection(nameof(SecretValues)).Bind(config));
.Configure<Options>(config => configuration.GetSection("Options").Bind(config))
.Configure<SecretValues>(config => configuration.GetSection(nameof(SecretValues)).Bind(config));

protected override ValueTask DisposeAsyncCore()
=> new();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
namespace Xunit.Microsoft.DependencyInjection.ExampleTests.Fixtures;

public class TestProjectFixtureWithoutAppsettings : TestBedFixture
{
protected override void AddServices(IServiceCollection services, IConfiguration configuration)
{
}

protected override ValueTask DisposeAsyncCore() => new();
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

public record SecretValues
{
public string? Secret1 { get; set; }
public string? Secret2 { get; set; }
public required string Secret1 { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

@ChrisDoernen the required keyword is unnecessary.

public required string Secret2 { get; set; }
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,45 @@

using System.Diagnostics;
using Microsoft.Extensions.Options;

namespace Xunit.Microsoft.DependencyInjection.ExampleTests;

public class UserSecretTests(ITestOutputHelper testOutputHelper, TestProjectFixture fixture) : TestBed<TestProjectFixture>(testOutputHelper, fixture)
public class UserSecretTests : TestBed<TestProjectFixture>
{
private static readonly Guid Guid = Guid.NewGuid(); // Ensure unique user secrets per test run

private readonly string _secret1Value = $"secret1-{Guid}";
private readonly string _secret2Value = $"secret2-{Guid}";

public UserSecretTests(ITestOutputHelper testOutputHelper, TestProjectFixture fixture) : base(testOutputHelper, fixture)
{
SetSecret(nameof(SecretValues.Secret1), _secret1Value);
SetSecret(nameof(SecretValues.Secret2), _secret2Value);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@ChrisDoernen Please remove this block.


[Fact]
public void TestSecretValues()
{
/*
* TODO: Create a user secret entry like the following payload in user secrets and remove the same from appsettings.json file:
Copy link
Contributor

Choose a reason for hiding this comment

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

@ChrisDoernen Please restore this block of comment

*
* "SecretValues": {
* "Secret1": "secret1value",
* "Secret2": "secret2value"
* }
*/
var secretValues = _fixture.GetService<IOptions<SecretValues>>(_testOutputHelper)!.Value;
Assert.NotEmpty(secretValues?.Secret1 ?? string.Empty);
Assert.NotEmpty(secretValues?.Secret1 ?? string.Empty);
Assert.Equal(secretValues.Secret1, _secret1Value);
Assert.Equal(secretValues.Secret2, _secret2Value);
}

private void SetSecret(string secretName, string secretValue)
Copy link
Contributor

Choose a reason for hiding this comment

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

@ChrisDoernen Unnecessary.

{
var startInfo = new ProcessStartInfo
{
FileName = "dotnet",
Arguments = $"user-secrets set {nameof(SecretValues)}:{secretName} {secretValue}",
WorkingDirectory = Path.Combine(Environment.CurrentDirectory, "..", "..", "..")
};
var proc = Process.Start(startInfo);
ArgumentNullException.ThrowIfNull(proc);

proc.WaitForExit();
if (proc.ExitCode != 0)
{
throw new Exception("Failed to set user secret");
}
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
{
"Options": {
"Rate": 10
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @ChrisDoernen we did not need to remove anything or alter anything related to using secret values. Can you please undo this change?
Let's put it in this way: I will only accept this PR if the changes are incremental. I do not want to get the already-implemented feature excluded or removed unless they are obsolete.

"SecretValues": {
"Secret1": "StoreSecret1InUserSecrets",
"Secret2": "StoreSecret2InUserSecrets"
}
}
11 changes: 4 additions & 7 deletions src/Abstracts/TestBedFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ public async ValueTask DisposeAsync()
/// <summary>
/// Adds services to the service collection. Called once before building the provider.
/// </summary>
protected abstract void AddServices(IServiceCollection services, IConfiguration? configuration);
protected abstract void AddServices(IServiceCollection services, IConfiguration configuration);

/// <summary>
/// Returns the test application settings descriptors (JSON files) to include.
/// </summary>
protected abstract IEnumerable<TestAppSettings> GetTestAppSettings();
protected virtual IEnumerable<TestAppSettings> GetTestAppSettings() => [];

/// <summary>
/// Override to asynchronously clean up resources created by the fixture.
Expand All @@ -147,13 +147,10 @@ protected virtual ILoggingBuilder AddLoggingProvider(ILoggingBuilder loggingBuil
/// </summary>
protected virtual void AddUserSecrets(IConfigurationBuilder configurationBuilder) { }

private IConfigurationRoot? GetConfigurationRoot()
private IConfigurationRoot GetConfigurationRoot()
{
var testAppSettings = GetTestAppSettings();
return
Copy link
Contributor

Choose a reason for hiding this comment

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

@ChrisDoernen what is the reason for removing the condition?

testAppSettings.All(setting => !string.IsNullOrEmpty(setting.Filename))
? GetConfigurationRoot(testAppSettings)
: default;
return GetConfigurationRoot(testAppSettings);
}

private IConfigurationRoot GetConfigurationRoot(IEnumerable<TestAppSettings> configurationFiles)
Expand Down