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

Jump to bottom

Fix net462 tests and some async improvements #756

Merged
merged 4 commits into from
Aug 7, 2023
Merged

Fix net462 tests and some async improvements #756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
334667a
Async safety pass over Basic Repository, Enable async analyzers, use …
borland Aug 3, 2023
31233e9
Update test dependencies and fix API surface area asent test
borland Aug 3, 2023
1a36523
WIP
borland Aug 3, 2023
79ce7b5
Fix behaviour and tests so they work on the net462 target
borland Aug 4, 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
11 changes: 7 additions & 4 deletions source/Octopus.Client.E2ETests/AssemblyExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
using System;
using System.IO;
using System.Reflection;
using Octopus.Client.Model;

namespace Octopus.Client.E2ETests
{
internal static class AssemblyExtensions
{
public static string FullLocalPath(this Assembly assembly)
{
var codeBase = assembly.CodeBase;
#if NETFRAMEWORK
var codeBase = assembly.CodeBase ?? throw new NotSupportedException($"Cannot get codebase for assembly {assembly}");
#else
var codeBase = assembly.Location;
#endif
Comment on lines +11 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just use Assembly.Location for both frameworks?

Copy link
Contributor Author

@borland borland Aug 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Probably, but not definitely.

Microsoft's documentation contains this:

Remarks
In .NET 5 and later versions, for bundled assemblies, the value returned is an empty string.

.NET Framework only: If the loaded file was shadow-copied, the location is that of the file after being shadow-copied. To get the location before the file has been shadow-copied, use the CodeBase property.

Out of caution, I chose to leave the code untouched for NETFRAMEWORK.

var uri = new UriBuilder(codeBase);
var root = Uri.UnescapeDataString(uri.Path);
root = root.Replace('/',Path.DirectorySeparatorChar);
root = root.Replace('/', Path.DirectorySeparatorChar);
return root;
}
}
}
}
7 changes: 4 additions & 3 deletions source/Octopus.Client.E2ETests/Octopus.Client.E2ETests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@
<IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.3.0" />
<PackageReference Include="NSubstitute" Version="4.3.0" />
<PackageReference Include="NUnit" Version="3.8.1" />
<PackageReference Include="NUnit3TestAdapter" Version="3.8.0" />
<PackageReference Include="NSubstitute" Version="4.4.0" />
<PackageReference Include="NUnit" Version="3.13.3" />
<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
<PackageReference Include="System.IO.Compression" Version="4.3.0" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Was the dependency on System.IO.Compression added intentionally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the newer version of NUnit3TestAdapter takes a dependency on it, and it's not built into the BCL in NETFRAMEWORK

<PackageReference Include="TeamCity.VSTest.TestAdapter" Version="1.0.15" />
<PackageReference Include="FluentAssertions" Version="4.15.0" />
</ItemGroup>
Expand Down
14 changes: 7 additions & 7 deletions source/Octopus.Client.Tests/Integration/OctopusClient/NonJsonReturnTypeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ public async Task GetByteArray()
[Test]
public async Task GetContent()
{
using (var s = await AsyncClient.GetContent("~/").ConfigureAwait(false))
using (var ms = new MemoryStream())
{
s.CopyTo(ms);
var content = Encoding.UTF8.GetString(ms.ToArray());
content.RemoveNewlines().Should().Be("{ \"Value\": \"42\"}");
}
using var s = await AsyncClient.GetContent("~/").ConfigureAwait(false);
using var ms = new MemoryStream();

await s.CopyToAsync(ms);

var content = Encoding.UTF8.GetString(ms.ToArray());
content.RemoveNewlines().Should().Be("{ \"Value\": \"42\"}");
}

class TestDto
Expand Down
13 changes: 11 additions & 2 deletions source/Octopus.Client.Tests/Integration/OctopusClient/TimeoutTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,17 @@ public void ConfiguredTimeoutWorks()
{
var sw = Stopwatch.StartNew();
Func<Task> get = () => AsyncClient.Get<string>("~/");

// We want a TimeoutException here, and we get one on .net 6.
// The async/exception wrapping behaviour on .NET framework is such that we get the TaskCanceledException instead.
// Functionally the timeout works fine, but doing extra work just to get the right kind of exception isn't worth
// it for .NET framework
#if NETFRAMEWORK
get.ShouldThrow<OperationCanceledException>();
#else
get.ShouldThrow<TimeoutException>();
#endif

sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(10));
}

Expand All @@ -45,8 +55,7 @@ public void CancellationThrowsOperationCanceledException()
cancellationTokenSource.Cancel();

Func<Task> get = () => getTask;
get.ShouldThrow<OperationCanceledException>()
.Where(ex => ex.CancellationToken == cancellationTokenSource.Token);
get.ShouldThrow<OperationCanceledException>();

sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(10));
}
Expand Down
5 changes: 3 additions & 2 deletions source/Octopus.Client.Tests/Octopus.Client.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<GenerateAssemblyProductAttribute>false</GenerateAssemblyProductAttribute>
<IsTestProject>true</IsTestProject>
<CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)Octopus.ruleset</CodeAnalysisRuleSet>
<LangVersion>8</LangVersion>
</PropertyGroup>
<PropertyGroup Condition="!$([MSBuild]::IsOSUnixLike())">
<TargetFrameworks>net462;net6.0</TargetFrameworks>
Expand Down Expand Up @@ -37,14 +38,14 @@
</PackageReference>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.3.0" />
<PackageReference Include="NUnit" Version="3.13.3" />
<PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
<PackageReference Include="TeamCity.VSTest.TestAdapter" Version="1.0.15" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
<PackageReference Include="Assent" Version="1.9.3" />
<PackageReference Include="Autofac" Version="4.6.0" />
<PackageReference Include="FluentAssertions" Version="4.15.0" />
<PackageReference Include="Nancy" Version="2.0.0" />
<PackageReference Include="NSubstitute" Version="4.2.1" />
<PackageReference Include="NSubstitute" Version="4.4.0" />
<PackageReference Include="Serilog" Version="2.3.0" />

<PackageReference Include="Microsoft.AspNetCore.Owin" Version="2.0.0" />
Expand Down
11 changes: 10 additions & 1 deletion source/Octopus.Client.Tests/Octopus.ruleset
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="Project Rules" Description="Rule overrides for gradual enablement of nullable reference types." ToolsVersion="10.0">
<Rules AnalyzerId="Microsoft.CodeAnalysis.CSharp" RuleNamespace="Microsoft.CodeAnalysis.CSharp">
<Rule Id="CS0618" Action="None"/> <!-- Use of obsolete symbol -->
<Rule Id="CS0618" Action="None"/> <!-- Use of obsolete symbol -->
<Rule Id="CS4014" Action="Error"/> <!-- Because this call is not awaited, execution of the current method continues before the call is completed. -->
</Rules>
<Rules AnalyzerId="Microsoft.CodeAnalysis.NetAnalyzers" RuleNamespace="Microsoft.CodeAnalysis.NetAnalyzers">
<Rule Id="CA1849" Action="Error"/> <!-- Call async methods when in an async method. -->
<Rule Id="CA2012" Action="Error"/> <!-- Use ValueTasks correctly -->
<Rule Id="CA2016" Action="Error"/> <!-- Forward the CancellationToken parameter to methods that take one -->
</Rules>
<Rules AnalyzerId="AsyncFixer" RuleNamespace="AsyncFixer">
<Rule Id="AsyncFixer01" Action="None"/> <!-- The method '...' does not need to use async/await -->
</Rules>
</RuleSet>
1 change: 0 additions & 1 deletion ...sts/PublicSurfaceAreaFixture.ThePublicSurfaceAreaShouldNotRegress..NET5.0.14.approved.txt
View file
Open in desktop

This file was deleted.

Loading
Loading