Extract Aspire.Hosting.MySql.Tests #4925

sebastienros · 2024-07-16T08:07:31Z

Contributes to #4294

Microsoft Reviewers: Open in CodeFlow

eerhardt

Can we also remove MySql from the EndToEnd tests?

If we do, we should probably have 2 functional tests - one for the non-EF component and one for the EF component. It could be in a single test if we wanted.

eerhardt · 2024-07-16T16:17:35Z

tests/Aspire.Hosting.MySql.Tests/Aspire.Hosting.MySql.Tests.csproj

+    <Compile Include="$(SharedDir)SecretsStore.cs" Link="Utils\SecretsStore.cs" />
+    <Compile Include="$(RepoRoot)src\Aspire.Hosting\ApplicationModel\UserSecretsParameterDefault.cs" Link="Utils\UserSecretsParameterDefault.cs" />


What are these for?

There are tests checking that this is the type of credential that is created with the resource.
For all DBs btw.

AddMySqlAddsGeneratedPasswordParameterWithUserSecretsParameterDefaultInRunMode
AddMySqlDoesNotAddGeneratedPasswordParameterWithUserSecretsParameterDefaultInPublishMode

These are adapted though since the actual instance type is from the internal type in the Hosting assembly, and not the local one. What would solve it would be to make the type public.

eerhardt · 2024-07-16T16:18:40Z

tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj

@@ -59,6 +57,7 @@
    <Compile Include="$(RepoRoot)src\Aspire.Hosting.Nats\NatsContainerImageTags.cs" />
    <Compile Include="$(RepoRoot)src\Aspire.Hosting.Milvus\MilvusContainerImageTags.cs" />
    <Compile Include="$(RepoRoot)src\Aspire.Hosting.MongoDB\MongoDBContainerImageTags.cs" />
+    <Compile Include="$(RepoRoot)src\Aspire.Hosting.MySql\MySqlContainerImageTags.cs" />


What is this still used for? It seems like it should be removed. Aspire.Hosting.Tests shouldn't need to be testing all the extension resources we create.

VerifyTestProgramFullManifest checks a manifest which has all these resources, and uses

"image": "{{TestConstants.AspireTestContainerRegistry}}/{{MySqlContainerImageTags.Image}}:{{MySqlContainerImageTags.Tag}}",

I think this line should be removed.

radical · 2024-07-16T19:02:22Z

MySqlConnector.MySqlException : Couldn't connect to server
---- MySqlConnector.MySqlEndOfStreamException : An incomplete response was received from the server

CallStack
   at MySqlConnector.Core.ServerSession.ConnectAsync(ConnectionSettings cs, MySqlConnection connection, Int64 startingTimestamp, ILoadBalancer loadBalancer, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ServerSession.cs:line 564
   at MySqlConnector.Core.ConnectionPool.ConnectSessionAsync(MySqlConnection connection, Action`4 logMessage, Int64 startingTimestamp, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ConnectionPool.cs:line 428
   at MySqlConnector.Core.ConnectionPool.ConnectSessionAsync(MySqlConnection connection, Action`4 logMessage, Int64 startingTimestamp, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ConnectionPool.cs:line 433
   at MySqlConnector.Core.ConnectionPool.GetSessionAsync(MySqlConnection connection, Int64 startingTimestamp, Int32 timeoutMilliseconds, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ConnectionPool.cs:line 111
   at MySqlConnector.Core.ConnectionPool.GetSessionAsync(MySqlConnection connection, Int64 startingTimestamp, Int32 timeoutMilliseconds, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ConnectionPool.cs:line 144
   at MySqlConnector.MySqlConnection.CreateSessionAsync(ConnectionPool pool, Int64 startingTimestamp, Activity activity, Nullable`1 ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlConnection.cs:line 1054
   at MySqlConnector.MySqlConnection.OpenAsync(Nullable`1 ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlConnection.cs:line 554
   at Aspire.Hosting.MySql.Tests.MySqlFunctionalTests.<>c__DisplayClass3_0.<<WithDataShouldPersistStateBetweenUsages>b__0>d.MoveNext() in /_/tests/Aspire.Hosting.MySql.Tests/MySqlFunctionalTests.cs:line 129
--- End of stack trace from previous location ---
   at Polly.ResiliencePipeline.<>c.<<ExecuteAsync>b__3_0>d.MoveNext()
--- End of stack trace from previous location ---
   at Polly.Outcome`1.GetResultOrRethrow()
   at Polly.ResiliencePipeline.ExecuteAsync(Func`2 callback, CancellationToken cancellationToken)
   at Aspire.Hosting.MySql.Tests.MySqlFunctionalTests.WithDataShouldPersistStateBetweenUsages(Boolean useVolume) in /_/tests/Aspire.Hosting.MySql.Tests/MySqlFunctionalTests.cs:line 126
   at Aspire.Hosting.MySql.Tests.MySqlFunctionalTests.WithDataShouldPersistStateBetweenUsages(Boolean useVolume) in /_/tests/Aspire.Hosting.MySql.Tests/MySqlFunctionalTests.cs:line 142
--- End of stack trace from previous location ---
----- Inner Stack Trace -----
   at MySqlConnector.Protocol.Serialization.ProtocolUtility.ReadPayloadAsync(BufferedByteReader bufferedByteReader, IByteHandler byteHandler, Func`1 getNextSequenceNumber, ArraySegmentHolder`1 previousPayloads, ProtocolErrorBehavior protocolErrorBehavior, IOBehavior ioBehavior) in /_/src/MySqlConnector/Protocol/Serialization/ProtocolUtility.cs:line 424
   at MySqlConnector.Core.ServerSession.ReceiveReplyAsync(IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ServerSession.cs:line 880
   at MySqlConnector.Core.ServerSession.ConnectAsync(ConnectionSettings cs, MySqlConnection connection, Int64 startingTimestamp, ILoadBalancer loadBalancer, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/ServerSession.cs:line 445

# Conflicts: # Aspire.sln

eerhardt · 2024-07-19T00:00:39Z

tests/Aspire.Hosting.MySql.Tests/Aspire.Hosting.MySql.Tests.csproj

+  <ItemGroup>
+    <Compile Include="$(SharedDir)SecretsStore.cs" Link="Utils\SecretsStore.cs" />
+    <Compile Include="$(RepoRoot)src\Aspire.Hosting\ApplicationModel\UserSecretsParameterDefault.cs" Link="Utils\UserSecretsParameterDefault.cs" />
+    <Compile Include="$(RepoRoot)src\Aspire.Hosting.Testing\ResourceLoggerForwarderService.cs" Link="Utils\ResourceLoggerForwarderService.cs" />


I don't think we should be compiling this into our tests. If we need this, we should just reference Aspire.Hosting.Testing.

Applied in another PR, copied here.

eerhardt · 2024-07-19T00:01:41Z

tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj

    <ProjectReference Include="..\..\src\Aspire.Hosting.Nats\Aspire.Hosting.Nats.csproj" IsAspireProjectResource="false" />
    <ProjectReference Include="..\..\src\Aspire.Hosting.Testing\Aspire.Hosting.Testing.csproj" IsAspireProjectResource="false" />
    <ProjectReference Include="..\Aspire.Components.Common.Tests\Aspire.Components.Common.Tests.csproj" IsAspireProjectResource="false" />
    <ProjectReference Include="..\testproject\TestProject.AppHost\TestProject.AppHost.csproj" IsAspireProjectResource="false" />
    <ProjectReference Include="..\Aspire.Hosting.Tests.SharedShim\Aspire.Hosting.Tests.SharedShim.csproj" IsAspireProjectResource="false" Aliases="AspireHostingShared" />
    <ProjectReference Include="..\..\src\Components\Aspire.Microsoft.Data.SqlClient\Aspire.Microsoft.Data.SqlClient.csproj" IsAspireProjectResource="false" />
    <ProjectReference Include="..\..\src\Components\Aspire.MongoDB.Driver\Aspire.MongoDB.Driver.csproj" IsAspireProjectResource="false" />
+    <ProjectReference Include="..\..\src\Components\Aspire.Npgsql\Aspire.Npgsql.csproj" IsAspireProjectResource="false" />


Why are we adding Npgsql in a MySql PR?

I think this should be removed.

eerhardt · 2024-07-19T00:01:58Z

tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj

@@ -59,6 +57,7 @@
    <Compile Include="$(RepoRoot)src\Aspire.Hosting.Nats\NatsContainerImageTags.cs" />
    <Compile Include="$(RepoRoot)src\Aspire.Hosting.Milvus\MilvusContainerImageTags.cs" />
    <Compile Include="$(RepoRoot)src\Aspire.Hosting.MongoDB\MongoDBContainerImageTags.cs" />
+    <Compile Include="$(RepoRoot)src\Aspire.Hosting.MySql\MySqlContainerImageTags.cs" />


I think this line should be removed.

eerhardt · 2024-07-19T00:02:43Z

tests/Aspire.Hosting.MySql.Tests/MySqlFunctionalTests.cs

+            [$"ConnectionStrings:{db.Resource.Name}"] = await db.Resource.ConnectionStringExpression.GetValueAsync(default)
+        });
+
+        hb.AddMySqlDataSource(db.Resource.Name);


Should we also have tests that use the EF component? That way we can remove MySql from the EndToEnd tests completely and not lose coverage.

Moved the EF tests here. Very similar to the one I did with Oracle (creating the tables using EF too, not just a SELECT 1 like in E2E).

eerhardt · 2024-07-19T00:05:40Z

tests/Aspire.Hosting.MySql.Tests/MySqlFunctionalTests.cs

+            var password = "p@ssw0rd1";
+
+            var passwordParameter = builder1.AddParameter("pwd");
+            builder1.Configuration["Parameters:pwd"] = password;
+            var mysql1 = builder1.AddMySql("mysql", passwordParameter).WithEnvironment("MYSQL_DATABASE", mySqlDbName);


Instead of hard-coding a password (and cause more cred scan issues), can we use ParameterResourceBuilderExtensions.CreateDefaultPasswordParameter instead?

aspire/src/Aspire.Hosting/ParameterResourceBuilderExtensions.cs

Lines 140 to 162 in e38416a

/// <summary>

/// Creates a default password parameter that generates a random password.

/// </summary>

/// <param name="builder">Distributed application builder</param>

/// <param name="name">Name of parameter resource</param>

/// <param name="lower"><see langword="true" /> if lowercase alphabet characters should be included; otherwise, <see langword="false" />.</param>

/// <param name="upper"><see langword="true" /> if uppercase alphabet characters should be included; otherwise, <see langword="false" />.</param>

/// <param name="numeric"><see langword="true" /> if numeric characters should be included; otherwise, <see langword="false" />.</param>

/// <param name="special"><see langword="true" /> if special characters should be included; otherwise, <see langword="false" />.</param>

/// <param name="minLower">The minimum number of lowercase characters in the result.</param>

/// <param name="minUpper">The minimum number of uppercase characters in the result.</param>

/// <param name="minNumeric">The minimum number of numeric characters in the result.</param>

/// <param name="minSpecial">The minimum number of special characters in the result.</param>

/// <returns>The created <see cref="ParameterResource"/>.</returns>

/// <remarks>

/// To ensure the generated password has enough entropy, see the remarks in <see cref="GenerateParameterDefault"/>.<br/>

/// The value will be saved to the app host project's user secrets store when <see cref="DistributedApplicationExecutionContext.IsRunMode"/> is <c>true</c>.

/// </remarks>

public static ParameterResource CreateDefaultPasswordParameter(

IDistributedApplicationBuilder builder, string name,

bool lower = true, bool upper = true, bool numeric = true, bool special = true,

int minLower = 0, int minUpper = 0, int minNumeric = 0, int minSpecial = 0)

{

…csproj

sebastienros · 2024-07-19T11:39:19Z

@eerhardt @radical fyi what was making the only test to fail was to use a bindmountpath which was not created before the container starts. It seems some containers are fine with that (Kafka?). But this would work locally just fine. Maybe some other kind of permissions, where locally the instance can create the folder, but not on the CI.

radical

Other than the one comment, LGTM 👍

radical · 2024-07-20T00:58:27Z

tests/Aspire.Hosting.MySql.Tests/MySqlFunctionalTests.cs

+            .AddRetry(new() { MaxRetryAttempts = 10, BackoffType = DelayBackoffType.Linear, Delay = TimeSpan.FromSeconds(2), ShouldHandle = new PredicateBuilder().Handle<MySqlException>() })
+            .Build();
+
+        var bindMountPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());


Would it be better to use Directory.CreateTempSubdirectory().FullName here too, and skip the need to delete+create the directory below?

* Extract Aspire.Hosting.MySql.Tests * Improve reliability * Record Docker logs * Fix ResourceLoggerForwarderService link in Aspire.Hosting.PostgreSQL.csproj * Remove E2E tests * Fix manifest test * Fix tests * Increase delay between retries * Create mount path directory * Address PR feedback * Use Directory.Delete * Use Linear backoff --------- Co-authored-by: Ankit Jain <[email protected]> Co-authored-by: Eric Erhardt <[email protected]> (cherry picked from commit 5238a73)

sebastienros requested a review from mitchdenny as a code owner July 16, 2024 08:07

sebastienros requested review from eerhardt and removed request for mitchdenny July 16, 2024 08:07

dotnet-issue-labeler bot added the area-codeflow for labeling automated codeflow. intentionally a different color! label Jul 16, 2024

Extract Aspire.Hosting.MySql.Tests

b8bfeb7

sebastienros force-pushed the sebros/mysqltests branch from 17e9c0c to b8bfeb7 Compare July 16, 2024 08:13

Merge remote-tracking branch 'origin/main' into sebros/mysqltests

ea9d1ee

sebastienros mentioned this pull request Jul 16, 2024

Break up the TestProject to individual projects per Resource/component #4294

Closed

20 tasks

eerhardt reviewed Jul 16, 2024

View reviewed changes

radical added 2 commits July 16, 2024 15:04

Merge remote-tracking branch 'origin/main' into sebros/mysqltests

8e92545

# Conflicts: # Aspire.sln

Merge remote-tracking branch 'origin/main' into sebros/mysqltests

9709401

# Conflicts: # Aspire.sln

radical added area-integrations Issues pertaining to Aspire Integrations packages and removed area-codeflow for labeling automated codeflow. intentionally a different color! labels Jul 16, 2024

sebastienros added 4 commits July 18, 2024 00:06

Improve reliability

18687e5

Merge branch 'main' into sebros/mysqltests

3cf5c1d

Merge branch 'main' into sebros/mysqltests

a003c3a

Record Docker logs

e1e1357

build-analysis bot mentioned this pull request Jul 18, 2024

Failing test Aspire.Dashboard.Tests.Integration.Playwright.AppBarTests.AppBar_Change_Theme #4851

Closed

radical mentioned this pull request Jul 18, 2024

Failing Aspire.Hosting.MySql.Tests.MySqlFunctionalTests.WithDataShouldPersistStateBetweenUsages #4973

Closed

build-analysis bot mentioned this pull request Jul 18, 2024

Two elasticsearch tests failing, blocking CI #4968

Closed

1 task

eerhardt reviewed Jul 19, 2024

View reviewed changes

sebastienros added 3 commits July 18, 2024 23:53

Merge remote-tracking branch 'origin/main' into sebros/mysqltests

009f08e

Fix ResourceLoggerForwarderService link in Aspire.Hosting.PostgreSQL.…

e88a5c8

…csproj

Remove E2E tests

ce649a8

sebastienros requested review from radical and ReubenBond as code owners July 19, 2024 08:02

sebastienros added 3 commits July 19, 2024 01:52

Fix manifest test

94984ec

Fix tests

86a0991

Increase delay between retries

391e527

Create mount path directory

c9b227a

eerhardt added 3 commits July 19, 2024 16:23

Address PR feedback

31bf91b

Use Directory.Delete

068144f

Use Linear backoff

521f6dd

eerhardt enabled auto-merge (squash) July 19, 2024 23:03

radical approved these changes Jul 20, 2024

View reviewed changes

eerhardt merged commit 5238a73 into main Jul 20, 2024
9 checks passed

eerhardt deleted the sebros/mysqltests branch July 20, 2024 01:00

radical mentioned this pull request Jul 26, 2024

[release/8.1] Extract Aspire.Hosting.Azure.Tests #5090

Closed

github-actions bot locked and limited conversation to collaborators Aug 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract Aspire.Hosting.MySql.Tests #4925

Extract Aspire.Hosting.MySql.Tests #4925

sebastienros commented Jul 16, 2024 •

edited by dotnet-policy-service bot

Loading

eerhardt left a comment

eerhardt Jul 16, 2024

sebastienros Jul 17, 2024

eerhardt Jul 16, 2024

sebastienros Jul 17, 2024

eerhardt Jul 19, 2024

radical commented Jul 16, 2024

eerhardt Jul 19, 2024

sebastienros Jul 19, 2024

eerhardt Jul 19, 2024

eerhardt Jul 19, 2024

eerhardt Jul 19, 2024

sebastienros Jul 19, 2024 •

edited

Loading

eerhardt Jul 19, 2024

sebastienros commented Jul 19, 2024 •

edited

Loading

radical left a comment

radical Jul 20, 2024

		<Compile Include="$(SharedDir)SecretsStore.cs" Link="Utils\SecretsStore.cs" />
		<Compile Include="$(RepoRoot)src\Aspire.Hosting\ApplicationModel\UserSecretsParameterDefault.cs" Link="Utils\UserSecretsParameterDefault.cs" />

	/// <summary>
	/// Creates a default password parameter that generates a random password.
	/// </summary>
	/// <param name="builder">Distributed application builder</param>
	/// <param name="name">Name of parameter resource</param>
	/// <param name="lower"><see langword="true" /> if lowercase alphabet characters should be included; otherwise, <see langword="false" />.</param>
	/// <param name="upper"><see langword="true" /> if uppercase alphabet characters should be included; otherwise, <see langword="false" />.</param>
	/// <param name="numeric"><see langword="true" /> if numeric characters should be included; otherwise, <see langword="false" />.</param>
	/// <param name="special"><see langword="true" /> if special characters should be included; otherwise, <see langword="false" />.</param>
	/// <param name="minLower">The minimum number of lowercase characters in the result.</param>
	/// <param name="minUpper">The minimum number of uppercase characters in the result.</param>
	/// <param name="minNumeric">The minimum number of numeric characters in the result.</param>
	/// <param name="minSpecial">The minimum number of special characters in the result.</param>
	/// <returns>The created <see cref="ParameterResource"/>.</returns>
	/// <remarks>
	/// To ensure the generated password has enough entropy, see the remarks in <see cref="GenerateParameterDefault"/>.<br/>
	/// The value will be saved to the app host project's user secrets store when <see cref="DistributedApplicationExecutionContext.IsRunMode"/> is <c>true</c>.
	/// </remarks>
	public static ParameterResource CreateDefaultPasswordParameter(
	IDistributedApplicationBuilder builder, string name,
	bool lower = true, bool upper = true, bool numeric = true, bool special = true,
	int minLower = 0, int minUpper = 0, int minNumeric = 0, int minSpecial = 0)
	{

Extract Aspire.Hosting.MySql.Tests #4925

Extract Aspire.Hosting.MySql.Tests #4925

Conversation

sebastienros commented Jul 16, 2024 • edited by dotnet-policy-service bot Loading

Microsoft Reviewers: Open in CodeFlow

eerhardt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radical commented Jul 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastienros Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastienros commented Jul 19, 2024 • edited Loading

radical left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastienros commented Jul 16, 2024 •

edited by dotnet-policy-service bot

Loading

sebastienros Jul 19, 2024 •

edited

Loading

sebastienros commented Jul 19, 2024 •

edited

Loading