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.

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

Re-enable Akka.Streams.Tests.TCK NUnit tests #4548

Merged
merged 9 commits into from Aug 21, 2020

Conversation

Arkatufus
Copy link
Contributor

@Arkatufus Arkatufus commented Aug 20, 2020

Some of the Pub-Sub part of Akka.Streams do not have their own standalone spec/unit tests and rely on Reactive.Streams.TCK for testing instead.
I'm re-enabling NUnit because this is quite dangerous.

NOTE:
Reactive.Streams.TCK only supports NUnit up to 3.9-ish because it relies on specific code behavior inside NUnit. Above a certain version, NUnit is not binary compatible with Reactive.Streams.TCK anymore.
DO NOT UPDATE THE NUNIT PACKAGE VERSION, UNLESS THIS HAS BEEN RESOLVED BY THE TCK DEVS

@Arkatufus
Copy link
Contributor Author

2020-08-20T21:09:31.9913360Z dotnet test -c Release --no-build --logger:trx --logger:"console;verbosity=normal" --framework net461 --results-directory "D:\a\1\s\TestResults" -- -parallel none
2020-08-20T21:09:32.8746579Z Test run for D:\a\1\s\src\core\Akka.Streams.Tests.TCK\bin\Release\net461\Akka.Streams.Tests.dll(.NETFramework,Version=v4.6.1)
2020-08-20T21:09:32.9939921Z Microsoft (R) Test Execution Command Line Tool Version 16.3.0
2020-08-20T21:09:32.9941609Z Copyright (c) Microsoft Corporation.  All rights reserved.
2020-08-20T21:09:32.9942247Z 
2020-08-20T21:09:33.1176512Z Starting test execution, please wait...
2020-08-20T21:09:33.1802951Z 
2020-08-20T21:09:33.1804293Z A total of 1 test files matched the specified pattern.
2020-08-20T21:09:34.6247562Z NUnit Adapter 3.17.0.0: Test execution started
2020-08-20T21:09:34.9014230Z Running all tests in D:\a\1\s\src\core\Akka.Streams.Tests.TCK\bin\Release\net461\Akka.Streams.Tests.dll
2020-08-20T21:09:36.6013857Z    NUnit3TestExecutor discovered 936 of 936 NUnit test cases
2020-08-20T21:09:39.0100105Z   √ Optional_spec104_mustSignalOnErrorWhenFails [530ms]
2020-08-20T21:09:39.0101621Z   √ Optional_spec105_emptyStreamMustTerminateBySignallingOnComplete [545ms]
2020-08-20T21:09:39.0293830Z Optional_spec111_maySupportMultiSubscribe: Skipped because tested publisher does NOT implement this OPTIONAL requirement.Reason for skipping was: Async error during test execution: ActorPublisher only supports one subscriber (which is allowed, see reactive-streams specification, rule 1.12)
< SNIP >
2020-08-20T21:13:38.6206228Z Untested_spec315_cancelMustNotThrowExceptionAndMustSignalOnError: Not verified using this TCK.
2020-08-20T21:13:38.6274175Z Untested_spec316_requestMustNotThrowExceptionAndMustOnErrorTheSubscriber: Not verified using this TCK.
2020-08-20T21:13:39.0064635Z NUnit Adapter 3.17.0.0: Test execution complete
2020-08-20T21:13:39.0582749Z [xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.4.1 (32-bit Desktop .NET 4.0.30319.42000)
2020-08-20T21:13:40.0982413Z   ! Untested_spec301_mustNotBeCalledOutsideSubscriberContext
2020-08-20T21:13:40.0983546Z   ! Untested_spec304_requestShouldNotPerformHeavyComputations [1ms]
2020-08-20T21:13:40.0984521Z   ! Untested_spec305_cancelMustNotSynchronouslyPerformHeavyCompuatation [1ms]
2020-08-20T21:13:40.0985347Z   ! Untested_spec310_requestMaySynchronouslyCallOnNextOnSubscriber
2020-08-20T21:13:40.0986480Z   ! Untested_spec311_requestMaySynchronouslyCallOnCompleteOrOnError
2020-08-20T21:13:40.0987218Z   ! Untested_spec314_cancelMayCauseThePublisherToShutdownIfNoOtherSubscriptionExists
2020-08-20T21:13:40.0988165Z   ! Untested_spec315_cancelMustNotThrowExceptionAndMustSignalOnError
2020-08-20T21:13:40.0989059Z   ! Untested_spec316_requestMustNotThrowExceptionAndMustOnErrorTheSubscriber
2020-08-20T21:13:43.0238489Z [xUnit.net 00:00:03.96]   Discovering: Akka.Streams.Tests
2020-08-20T21:13:43.2518918Z [xUnit.net 00:00:04.19]   Discovered:  Akka.Streams.Tests
2020-08-20T21:13:43.6703395Z Results File: D:\a\1\s\TestResults\VssAdministrator_WIN-94OD2NJ61CK_2020-08-20_21_09_38.trx
2020-08-20T21:13:43.6705017Z 
2020-08-20T21:13:43.6706063Z Test Run Successful.
2020-08-20T21:13:43.6706653Z Total tests: 936
2020-08-20T21:13:43.6707333Z      Passed: 516
2020-08-20T21:13:43.6708599Z     Skipped: 420
2020-08-20T21:13:43.6710889Z  Total time: 4.1704 Minutes

@Aaronontheweb Aaronontheweb added this to the 1.4.11 milestone Aug 20, 2020
<PackageReference Include="Reactive.Streams.TCK" Version="1.0.2" />
<PackageReference Include="NUnit" Version="3.12.0" />
<!-- !!!WARNING!!! ALWAYS MATCH NUNIT VERSION WITH THE VERSION USED BY REACTIVE TCK !!!WARNING!!! -->
<PackageReference Include="NUnit" Version="3.7.1" />
Copy link
Member

Choose a reason for hiding this comment

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

If we don't explicitly include an NUnit reference here, we'll take whatever NUnit version is used by Reactive.Streams.TCK as a transitive dependency. So maybe the safest way to handle this is to not explicitly specify an NUnit dependency at all? What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I'll do a quick check and update.

@Aaronontheweb
Copy link
Member

https://dev.azure.com/dotnet/Akka.NET/_build/results?buildId=37591&view=logs&j=59d8d678-3f9b-55ef-c67e-99309e0aad31&t=ca366254-23e0-5149-5105-de91bebc95cc&l=11

2020-08-20T21:09:55.6666693Z ##[warning]No test result files matching **/*.trx were found.

Is there a way via dotnet test to get this output from the NUnit adapter, just like we do with XUnit today?

@Arkatufus
Copy link
Contributor Author

No idea, I forgot about the trx part, let me check...

@Arkatufus
Copy link
Contributor Author

The test assembly was set to Akka.Stream.Tests so it got picked up wrong and run under the Xunit test which causes a lot of weird things to happen. The test log should've flagged it, but I missed it. The build script skipped over it because it had TCK after the Tests keyword.

@Aaronontheweb
Copy link
Member

@Arkatufus the tests ran fine on .NET Framework, but looks like they never ran at all on .NET Core for either Windows or Linux

@Arkatufus
Copy link
Contributor Author

Yes, I removed .NET core from TargetFrameworks because Reactive.Streams.TCK only supports net45

<!-- !!!WARNING!!!
NUNIT VERSION HAVE TO MATCH WITH THE VERSION USED BY REACTIVE TCK.
!!!WARNING!!! -->
<PackageReference Include="NUnit" Version="3.7.1" Condition="'$(TargetFramework)' == '$(NetCoreTestVersion)'"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to add this back in, because .NET Core could not use the NUnit package referenced inside the TCK package

@Aaronontheweb Aaronontheweb merged commit ebe5fd9 into akkadotnet:dev Aug 21, 2020
@Aaronontheweb
Copy link
Member

nice work @Arkatufus

@Arkatufus Arkatufus deleted the Add_NUnit_to_build_system branch March 8, 2021 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants