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.

Sign up for GitHub

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

Use TestContainer Cloud on CI for windows tests that can not host the requested images #2139

Merged
merged 8 commits into from
Nov 21, 2023
Merged

Use TestContainer Cloud on CI for windows tests that can not host the requested images #2139

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
55b9de6
Reenable profiler tests on windows
Mpdreamz Jul 25, 2023
3c95ea0
reenable other docker tests that were previously disabled on windows
Mpdreamz Jul 27, 2023
669c6ac
ci: try to setup testcontainers cloud
amannocci Sep 22, 2023
73165f7
fix: use correct import for docker facts
amannocci Sep 22, 2023
7a84757
ensure test container cloud is setup for regular tests run too
Mpdreamz Oct 10, 2023
24fe6aa
increase logging of SqlClientListenerTests
Mpdreamz Oct 19, 2023
1a15e9a
Merge remote-tracking branch 'origin/main' into feature/windows-profi…
Mpdreamz Nov 16, 2023
9f97169
Merge branch 'main' into feature/windows-profiler-tests
Mpdreamz Nov 16, 2023
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
14 changes: 12 additions & 2 deletions .github/workflows/test-windows.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ jobs:
- name: Build
run: dotnet build -c Release --verbosity minimal

- name: Setup Testcontainers Cloud Client
uses: atomicjar/testcontainers-cloud-setup-action@v1
with:
token: ${{ secrets.TC_CLOUD_TOKEN }}

Comment on lines +43 to +47
Copy link
Member

@v1v v1v Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm afraid this won't work for forked PRs. Shall this be changed by enabling only when working on the upstream rather than on forked-PRs?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would be great @v1v 👍

Copy link
Member

Choose a reason for hiding this comment

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

Cool, I'll code something. Then likely, I might need your help with skipping those tests for forked PRs.

- name: 'Tests: Unit'
uses: ./.github/workflows/test
with:
Expand All @@ -57,7 +62,7 @@ jobs:

- name: Build agent-zip
run: ./build.bat agent-zip

- name: 'Tests: StartupHooks'
uses: ./.github/workflows/test
with:
Expand All @@ -67,7 +72,7 @@ jobs:
profiler-tests:
runs-on: windows-2022
# Disable profiler tests for now
if: ${{ false }}
# if: ${{ false }}
steps:
- uses: actions/checkout@v3
- name: Bootstrap Action Workspace
Expand All @@ -78,6 +83,11 @@ jobs:
- name: Build profiler
run: ./build.bat profiler-zip

- name: Setup Testcontainers Cloud Client
uses: atomicjar/testcontainers-cloud-setup-action@v1
with:
token: ${{ secrets.TC_CLOUD_TOKEN }}

- name: 'Tests: Profiler'
uses: ./.github/workflows/test
with:
Expand Down
6 changes: 0 additions & 6 deletions test/Elastic.Apm.Tests.Utilities/Docker/DockerFactAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ static DockerFactAttribute()
{
try
{
if (TestEnvironment.IsCi && TestEnvironment.IsWindows)
{
_skip = "not running tests that require docker in CI on Windows";
return;
}

var result = Proc.Start(new StartArguments("docker", "--version"));
if (result.ExitCode != 0)
_skip = "docker not installed";
Expand Down
6 changes: 0 additions & 6 deletions test/Elastic.Apm.Tests.Utilities/Docker/DockerTheoryAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ static DockerTheoryAttribute()
{
try
{
if (TestEnvironment.IsCi && TestEnvironment.IsWindows)
{
_skip = "not running tests that require docker in CI on Windows";
return;
}

var result = Proc.Start(new StartArguments("docker", "--version"));
if (result.ExitCode != 0)
_skip = "docker not installed";
Expand Down
60 changes: 0 additions & 60 deletions test/Elastic.Apm.Tests.Utilities/XUnit/DisabledOnWindowsFact.cs
View file
Open in desktop

This file was deleted.

17 changes: 17 additions & 0 deletions test/Elastic.Apm.Tests.Utilities/XUnit/DisabledOnWindowsTheory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to Elasticsearch B.V under
// one or more agreements.
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information

using Xunit;

namespace Elastic.Apm.Tests.Utilities.XUnit;

public sealed class DisabledOnWindowsTheory : TheoryAttribute
{
public DisabledOnWindowsTheory()
{
if (TestEnvironment.IsWindows)
Skip = "This test is disabled on Windows.";
}
}
5 changes: 3 additions & 2 deletions test/instrumentations/Elastic.Apm.Elasticsearch.Tests/ElasticsearchTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Elastic.Apm.Api;
using Elastic.Apm.DiagnosticSource;
using Elastic.Apm.Tests.Utilities;
using Elastic.Apm.Tests.Utilities.Docker;
using Elastic.Apm.Tests.Utilities.XUnit;
using Elasticsearch.Net;
using FluentAssertions;
Expand All @@ -28,7 +29,7 @@ public ElasticsearchTests(ElasticsearchFixture fixture)
_client = new ElasticLowLevelClient(settings);
}

[DisabledOnWindowsGitHubActionsDockerFact]
[DockerFact]
public async Task Elasticsearch_Span_Should_Align_With_Spec()
{
var payloadSender = new MockPayloadSender();
Expand Down Expand Up @@ -86,7 +87,7 @@ private static void AssertSpan(ISpan span)
span.Context.Destination.Port.Should().BeGreaterThan(0).And.BeLessThan(65536);
}

[DisabledOnWindowsGitHubActionsDockerFact]
[DockerFact]
public async Task Elasticsearch_Span_Does_Not_Have_Http_Child_Span()
{
var payloadSender = new MockPayloadSender();
Expand Down
5 changes: 3 additions & 2 deletions test/instrumentations/Elastic.Apm.MongoDb.Tests/MongoApmTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Elastic.Apm.Api;
using Elastic.Apm.MongoDb.Tests.Fixture;
using Elastic.Apm.Tests.Utilities;
using Elastic.Apm.Tests.Utilities.Docker;
using Elastic.Apm.Tests.Utilities.XUnit;
using FluentAssertions;
using MongoDB.Bson;
Expand Down Expand Up @@ -64,7 +65,7 @@ public Task DisposeAsync(IMongoCollection<BsonDocument> collection) =>

private readonly MockPayloadSender _payloadSender;

[DisabledOnWindowsFact]
[DockerFact]
public async Task ApmAgent_ShouldCorrectlyCaptureSpan()
{
// Arrange
Expand Down Expand Up @@ -101,7 +102,7 @@ public async Task ApmAgent_ShouldCorrectlyCaptureSpan()
});
}

[DisabledOnWindowsFact]
[DockerFact]
public async Task ApmAgent_ShouldCorrectlyCaptureSpanAndError_WhenMongoCommandFailed()
{
// Arrange
Expand Down
3 changes: 2 additions & 1 deletion test/instrumentations/Elastic.Apm.SqlClient.Tests/EfCoreWithMsSqlTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Elastic.Apm.EntityFrameworkCore;
using Elastic.Apm.Instrumentations.SqlClient;
using Elastic.Apm.Tests.Utilities;
using Elastic.Apm.Tests.Utilities.Docker;
using Elastic.Apm.Tests.Utilities.XUnit;
using FluentAssertions;
using Microsoft.EntityFrameworkCore;
Expand Down Expand Up @@ -43,7 +44,7 @@ public EfCoreWithMsSqlTests(ITestOutputHelper testOutputHelper, SqlServerFixture
/// Executes a db query within a transaction while both SqlClient and EFCore capturing is active.
/// Makes sure that the db call is only captured once - so only 1 of them captures the call, the other one ignores it.
/// </summary>
[DisabledOnWindowsFact]
[DockerFact]
public void BothEfCoreAndSqlClientCapturingActive()
{
var dbContextOptionsBuilder = new DbContextOptionsBuilder<TestDbContext>();
Expand Down
4 changes: 4 additions & 0 deletions test/instrumentations/Elastic.Apm.SqlClient.Tests/Elastic.Apm.SqlClient.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,8 @@
<ProjectReference Include="$(SolutionRoot)\test\Elastic.Apm.Tests.Utilities\Elastic.Apm.Tests.Utilities.csproj" />
</ItemGroup>

<ItemGroup>
<Content Include="..\..\xunit.runner.json" Link="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>

</Project>
12 changes: 7 additions & 5 deletions test/instrumentations/Elastic.Apm.SqlClient.Tests/SqlClientListenerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Elastic.Apm.Api;
using Elastic.Apm.Instrumentations.SqlClient;
using Elastic.Apm.Tests.Utilities;
using Elastic.Apm.Tests.Utilities.Docker;
using Elastic.Apm.Tests.Utilities.XUnit;
using FluentAssertions;
using Xunit;
Expand Down Expand Up @@ -38,9 +39,10 @@ public SqlClientListenerTests(ITestOutputHelper testOutputHelper, SqlServerFixtu

_testOutputHelper = testOutputHelper;

_payloadSender = new MockPayloadSender();
var logger = new LineWriterToLoggerAdaptor(new XunitOutputToLineWriterAdaptor(_testOutputHelper));
_payloadSender = new MockPayloadSender(logger);
_apmAgent = new ApmAgent(new TestAgentComponents(
new LineWriterToLoggerAdaptor(new XunitOutputToLineWriterAdaptor(_testOutputHelper)),
logger,
payloadSender: _payloadSender));
_subscription = _apmAgent.Subscribe(new SqlClientDiagnosticSubscriber());
}
Expand All @@ -63,7 +65,7 @@ public static IEnumerable<object[]> Connections
}
}

[DisabledOnWindowsTheory]
[DockerTheory]
[MemberData(nameof(Connections))]
public async Task SqlClientDiagnosticListener_ShouldCaptureSpan(string providerName, Func<string, DbConnection> connectionCreator)
{
Expand Down Expand Up @@ -117,7 +119,7 @@ await _apmAgent.Tracer.CaptureTransaction("transaction", "type", async _ =>
span.Context.Service.Target.Name.Should().Be(span.Context.Db.Instance);
}

[DisabledOnWindowsTheory]
[DockerTheory]
[MemberData(nameof(Connections))]
public async Task SqlClientDiagnosticListener_ShouldCaptureErrorFromSystemSqlClient(string providerName,
Func<string, DbConnection> connectionCreator
Expand Down Expand Up @@ -181,7 +183,7 @@ await _apmAgent.Tracer.CaptureTransaction("transaction", "type", async _ =>
span.Context.Service.Target.Name.Should().Be(span.Context.Db.Instance);
}

[DisabledOnWindowsTheory]
[DockerTheory]
[MemberData(nameof(Connections))]
public async Task SqlClientDiagnosticListener_ShouldNotUseCumulativeDurations(string providerName, Func<string, DbConnection> connectionCreator)
{
Expand Down
23 changes: 20 additions & 3 deletions test/instrumentations/Elastic.Apm.SqlClient.Tests/SqlServerFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,35 @@
using System.Threading.Tasks;
using Testcontainers.MsSql;
using Xunit;
using Xunit.Abstractions;
using Xunit.Sdk;

namespace Elastic.Apm.SqlClient.Tests
{
// ReSharper disable once ClassNeverInstantiated.Global - it's used as a generic parameter
public sealed class SqlServerFixture : IAsyncLifetime
{
private readonly MsSqlContainer _container = new MsSqlBuilder().Build();
private readonly MsSqlContainer _container;
private readonly IMessageSink _sink;

public string ConnectionString => _container.GetConnectionString();

public Task InitializeAsync() => _container.StartAsync();
public SqlServerFixture(IMessageSink sink)
{
_sink = sink;
_container = new MsSqlBuilder()
.Build();
}

public Task DisposeAsync() => _container.DisposeAsync().AsTask();
public async Task InitializeAsync()
{
await _container.StartAsync();
var (stdOut, stdErr) = await _container.GetLogsAsync();

_sink.OnMessage(new DiagnosticMessage(stdOut));
_sink.OnMessage(new DiagnosticMessage(stdErr));
}

public async Task DisposeAsync() => await _container.DisposeAsync();
}
}
7 changes: 4 additions & 3 deletions test/instrumentations/Elastic.Apm.StackExchange.Redis.Tests/ProfilingSessionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,17 @@
using System.Threading.Tasks;
using Elastic.Apm.Api;
using Elastic.Apm.Tests.Utilities;
using Elastic.Apm.Tests.Utilities.XUnit;
using Elastic.Apm.Tests.Utilities.Docker;
using FluentAssertions;
using StackExchange.Redis;
using Testcontainers.Redis;
using Xunit;

namespace Elastic.Apm.StackExchange.Redis.Tests
{
public class ProfilingSessionTests
{
[DisabledOnWindowsGitHubActionsDockerFact]
[DockerFact]
public async Task Capture_Redis_Commands_On_Transaction()
{
await using var container = new RedisBuilder().Build();
Expand Down Expand Up @@ -75,7 +76,7 @@ await agent.Tracer.CaptureTransaction("Set and Get String", ApiConstants.TypeDb,
await container.StopAsync();
}

[DisabledOnWindowsGitHubActionsDockerFact]
[DockerFact]
public async Task Capture_Redis_Commands_On_Span()
{
await using var container = new RedisBuilder().Build();
Expand Down
13 changes: 7 additions & 6 deletions test/instrumentations/Elastic.Clients.Elasticsearch.Tests/ElasticsearchTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
using Elastic.Apm.DiagnosticSource;
using Elastic.Apm.Elasticsearch;
using Elastic.Apm.Tests.Utilities;
using Elastic.Apm.Tests.Utilities.XUnit;
using Elastic.Apm.Tests.Utilities.Docker;
using FluentAssertions;
using Xunit;
using Xunit.Abstractions;
Expand All @@ -32,7 +32,7 @@ public ElasticsearchTests(ITestOutputHelper testOutputHelper, ElasticsearchTestF
_client = _esClientListenerFixture.Client ?? throw new Exception("ElasticsearchClient is `null`");
}

[DisabledOnWindowsGitHubActionsDockerFact]
[DockerFact]
public async Task IndexDataTest()
{
var (payloadSender, apmAgent) = SetUpAgent();
Expand All @@ -56,7 +56,8 @@ public async Task IndexDataTest()
elasticsearchSpan.Otel.Attributes.Should().Contain(new KeyValuePair<string, object>("net.peer.name", _esClientListenerFixture.Container.Hostname));
}

[DisabledOnWindowsGitHubActionsDockerFact]

[DockerFact]
public async Task GetDocumentTest()
{
// make sure data is present
Expand All @@ -83,7 +84,7 @@ public async Task GetDocumentTest()
elasticsearchSpan.Otel.Attributes.Should().Contain(new KeyValuePair<string, object>("net.peer.name", _esClientListenerFixture.Container.Hostname));
}

[DisabledOnWindowsGitHubActionsDockerFact]
[DockerFact]
public async Task SearchDocumentTest()
{
// make sure data is present
Expand All @@ -110,7 +111,7 @@ public async Task SearchDocumentTest()
elasticsearchSpan.Otel.Attributes.Should().Contain(new KeyValuePair<string, object>("net.peer.name", _esClientListenerFixture.Container.Hostname));
}

[DisabledOnWindowsGitHubActionsDockerFact]
[DockerFact]
public async Task UpdateDocumentTest()
{
// make sure data is present
Expand Down Expand Up @@ -147,7 +148,7 @@ await apmAgent.Tracer.CaptureTransaction("Test", "Foo", async () =>
elasticsearchSpan.Otel.Attributes.Should().Contain(new KeyValuePair<string, object>("net.peer.name", _esClientListenerFixture.Container.Hostname));
}

[DisabledOnWindowsGitHubActionsDockerFact]
[DockerFact]
public async Task DeleteDocumentTest()
{
// make sure data is present
Expand Down
Loading