[tests] Enable `Aspire.Hosting.Testing`, and `Aspire.Hosting` tests that use docker to run on the build machine by radical · Pull Request #4500 · dotnet/aspire

radical · 2024-06-13T17:14:14Z

There is work in progress to get tests like Aspire.Hosting.Testing, and Aspire.Hosting running on helix. Meanwhile, we can have the tests run on the build machine, at least for linux which has docker installed, by simply removing/updating the test attributes blocking them.

Remove [LocalOnlyFact/Theory] attributes in favor of [RequiresDocker]
Remove unncessary SkipOnHelix attributes from SchemaTests
Install devcerts on Linux/build machine
Fix path construction for nodeapp so it correctly works on linux
Split, and fix node/npm tests to work on CI whenever the relevant tool is available.

Microsoft Reviewers: Open in CodeFlow

.. on windows/build-machine on CI.

…ing can work To be specific, `category!=failing` is required.

- this allows the tests to run on the build machine

…gging issues

radical · 2024-06-13T21:49:35Z

tests/Aspire.Hosting.Testing.Tests/TestingFactoryTests.cs

    public async Task HttpClientGetTest()
    {
        var httpClient = _app.CreateHttpClient("mywebapp1");
+        await Task.Delay(3000);


@ReubenBond @DamianEdwards This PR enables the hosting.testing tests to run on CI. But two tests fail - HttpClientGetTest, and SelectsFirstLaunchProfile with connection refused. And it seems to be due to webapp not being ready. I added a Task.delay to confirm that, and it does work. But what is the correct way to have the test run only once the webapp is ready?

You should configure the HttpClient with resilience so that it retries. Once #4444 goes in you'll be able to at least wait for the resource to transition to the "Running" state before attempting to send the request, but you still need the retries.

Should that be added to the Aspire.Hosting.Testing api? I added that as an extension method in the test right now.

HttpClient default resiliency pipelines aren't composable so we don't add it by default as there'd be no way to remove or modify it. The test templates have code in them that adds resiliency in the test itself.

Looking at the template, I added this to the TestingAppHost1.AppHost/Program.cs:

builder.Services.ConfigureHttpClientDefaults(http => { http.AddStandardResilienceHandler(); });

radical · 2024-06-13T22:58:52Z

tests/testproject/TestProject.AppHost/TestProgram.cs

            // Relative to this project so that it doesn't changed based on
            // where this code is referenced from.
-            var path = Path.Combine(Projects.TestProject_AppHost.ProjectPath, @"..\nodeapp");
+            var path = Path.Combine(Projects.TestProject_AppHost.ProjectPath, "..", "nodeapp");


If the nodeapp fails to start for some reason, then no error is being surfaced. @ReubenBond

… to wait for all the endpoints to get allocated

radical · 2024-06-13T23:50:58Z

tests/Aspire.Hosting.Tests/DistributedApplicationTests.cs


        var token = new CancellationTokenSource(TimeSpan.FromMinutes(1)).Token;

+        await Task.Delay(5000);


@ReubenBond I added a delay here because the first client.GetStringAsync($"{httpEndPoint}urls" returned only the http endpoint. But the second invocation got both http, and https. Does this need to wait for all the endpoints to get allocated?

Should the API surface something (event?) for the endpoints getting allocated?
For this test, we could either use IDistributedApplicationLifecycleHook.AfterEndpointsAllocatedAsync or retry a few times in a loop.

The tests have an existing class for the hook which I tried to use here - https://gist.github.com/radical/7ef257229178c36f6b72ffd09f7e0b79#file-test-cs-L22-L27

But running the test with CWLs added to print the results from /urls:

Got urls from http://localhost:1234/urls: ["http://localhost:1234"] Got urls from https://localhost:57048/urls: ["http://localhost:1234","https://localhost:57051"]

The first call is missing the https endpoint. Did the AfterEndpointsAllocatedAsync get invoked too early? Or am I missing something else?
@ReubenBond @DamianEdwards

tests/Aspire.Hosting.Testing.Tests/DistributedApplicationHttpClientExtensionsForTests.cs

tests/Aspire.Hosting.Testing.Tests/TestingBuilderTests.cs

tests/TestingAppHost1/TestingAppHost1.AppHost/TestingAppHost1.AppHost.csproj

…AppHost.csproj Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

…build 0.5.3 (dotnet#4598) [main] Update dependencies from microsoft/usvc-apiserver

…gs. (dotnet#4601)

…t by using httpclient with resilience

tests/Aspire.Components.Common.Tests/RequiresDockerAttribute.cs

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

eerhardt

LGTM. Thanks!

radical · 2024-06-20T21:58:00Z

Added some debug bits, so don't merge before that is removed.

This reverts commit cb206dc.

dotnet-bot · 2024-06-21T00:13:27Z

‼️ Found issues ‼️

Project	Coverage Type	Expected	Actual
Aspire.RabbitMQ.Client	Branch	80	77.5 🔻

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=714974&view=codecoverage-tab

radical · 2024-06-21T00:36:28Z

How did the coverage go down here?!

‼️ Found issues ‼️

Project Coverage Type Expected Actual
Aspire.RabbitMQ.Client Branch 80 77.5 🔻
Full code coverage report: dev.azure.com/dnceng-public/public/_build/results?buildId=714974&view=codecoverage-tab

How did the coverage go down here?!

radical · 2024-06-21T00:41:26Z

The number from the previous main build is 75%, so this should be an increase. Though, again, I'm not sure why with this PR. Maybe some extra bits are getting covered in the docker dependent tests.

cc @RussKie

RussKie · 2024-06-21T00:49:28Z

The number from the previous main build is 75%, so this should be an increase. Though, again, I'm not sure why with this PR. Maybe some extra bits are getting covered in the docker dependent tests.

It could be due to the code coverage reporter not correctly reporting the numbers... ¯\_(ツ)_/¯ You'd need to look into the actual cobertura.xml.

https://dev.azure.com/dnceng-public/public/_build/results?buildId=714972&view=logs&j=8cf0092d-1f6c-522b-3c27-9f632d33f4f3&t=b92e38fc-0165-5b68-b32b-2eef48c1a225&l=9

The thing is - the new version was broken and unusable, and I made numerous attempt to contact the team responsible in the past two years but they wouldn't listen....

ghost added the area-integrations Issues pertaining to Aspire Integrations packages label Jun 13, 2024

radical added 7 commits June 13, 2024 17:34

[tests] RequiresDocker: Update condition to disallow docker based tests

9c964a6

.. on windows/build-machine on CI.

[tests] Use .runsettings when running non-helix tests, so tests skipp…

3d8949c

…ing can work To be specific, `category!=failing` is required.

[tests] Use RequiresDocker instead of LocalOnlyFact/Theory attributes

fc2bf4f

- this allows the tests to run on the build machine

[tests] Install devcerts on linux/CI

78dc7a2

[tests] Throw if application hasn't been initialized, useful for debu…

ca5d62f

…gging issues

[tests] FIXME: add delay to wait for the webapp to be ready

cbc7897

[tests] Fix VerifyNodeAppWorks by fixing the constructed path to nodeapp

a85b721

radical force-pushed the hosting-testing-build branch from 37f978c to a85b721 Compare June 13, 2024 21:44

radical changed the title ~~[tests] Enable docker based tests on build machine~~ [tests] Enable Aspire.Hosting.Testing, and Aspire.Hosting tests using docker to run on the build machine Jun 13, 2024

radical commented Jun 13, 2024

View reviewed changes

radical changed the title ~~[tests] Enable Aspire.Hosting.Testing, and Aspire.Hosting tests using docker to run on the build machine~~ [tests] Enable Aspire.Hosting.Testing, and Aspire.Hosting tests that use docker to run on the build machine Jun 13, 2024

radical marked this pull request as ready for review June 13, 2024 21:51

radical requested review from DamianEdwards, ReubenBond, davidfowl and joperezr June 13, 2024 21:51

radical commented Jun 13, 2024

View reviewed changes

radical requested a review from eerhardt June 13, 2024 23:45

radical marked this pull request as draft June 13, 2024 23:45

FIXME: add delay in ProxylessAndProxiedEndpointBothWorkOnSameResource…

502e36f

… to wait for all the endpoints to get allocated

radical commented Jun 13, 2024

View reviewed changes

radical requested a review from mitchdenny June 13, 2024 23:53

radical added 3 commits June 13, 2024 21:05

[tests] Split npm/nodeapp test

106ed06

[tests] Remove RequiresDocker from Hosting.Tests that don't need it

6913816

cleanup

daccd46

radical mentioned this pull request Jun 14, 2024

[tests] Install node+npm on CI build machines #4508

Open

Disable node/npm tests on CI as the binaries are not available

f4c4a77

radical removed request for ReubenBond and davidfowl June 14, 2024 02:09

Use b.AddStandardResilienceHandler

fe32fd3

radical marked this pull request as ready for review June 20, 2024 01:53

radical requested review from ReubenBond, joperezr and mitchdenny June 20, 2024 01:53

eerhardt reviewed Jun 20, 2024

View reviewed changes

tests/Aspire.Hosting.Testing.Tests/DistributedApplicationHttpClientExtensionsForTests.cs Outdated Show resolved Hide resolved

eerhardt reviewed Jun 20, 2024

View reviewed changes

tests/Aspire.Hosting.Testing.Tests/TestingBuilderTests.cs Show resolved Hide resolved

eerhardt reviewed Jun 20, 2024

View reviewed changes

tests/TestingAppHost1/TestingAppHost1.AppHost/TestingAppHost1.AppHost.csproj Outdated Show resolved Hide resolved

radical and others added 7 commits June 20, 2024 14:12

Update tests/TestingAppHost1/TestingAppHost1.AppHost/TestingAppHost1.…

fd2474e

…AppHost.csproj Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

Update dependencies from https://github.com/microsoft/usvc-apiserver …

023fd5c

…build 0.5.3 (dotnet#4598) [main] Update dependencies from microsoft/usvc-apiserver

Update additional DCP resource schema (dotnet#4600)

f9e2eba

Update container logs available condition to allow streaming build lo…

b1b0682

…gs. (dotnet#4601)

address review feedback from @ eerhardt

ac52e88

Merge remote-tracking branch 'origin/main' into hosting-testing-build

251d59a

Fix Aspire.Hosting.Testing.Tests.TestingFactoryTests.HttpClientGetTes…

14b5e2f

…t by using httpclient with resilience

eerhardt reviewed Jun 20, 2024

View reviewed changes

tests/Aspire.Components.Common.Tests/RequiresDockerAttribute.cs Outdated Show resolved Hide resolved

Update tests/Aspire.Components.Common.Tests/RequiresDockerAttribute.cs

2f8026c

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

eerhardt approved these changes Jun 20, 2024

View reviewed changes

radical added 2 commits June 20, 2024 16:28

Merge branch 'dotnet:main' into hosting-testing-build

63bb21e

debug dcp failure

cb206dc

Revert "debug dcp failure"

9db8eee

This reverts commit cb206dc.

radical merged commit fb97eda into dotnet:main Jun 21, 2024

radical deleted the hosting-testing-build branch June 21, 2024 00:35

radical mentioned this pull request Jun 21, 2024

Updates to Aspire.Hosting.Testing #4444

Merged

github-actions bot locked and limited conversation to collaborators Jul 21, 2024


		var token = new CancellationTokenSource(TimeSpan.FromMinutes(1)).Token;

		await Task.Delay(5000);

Conversation

radical commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radical Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eerhardt left a comment

Choose a reason for hiding this comment

Uh oh!

radical commented Jun 20, 2024

Uh oh!

dotnet-bot commented Jun 21, 2024

Uh oh!

radical commented Jun 21, 2024

Uh oh!

radical commented Jun 21, 2024

Uh oh!

RussKie commented Jun 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

radical commented Jun 13, 2024 •

edited

Loading

radical Jun 14, 2024 •

edited

Loading