Fix recently added AndroidMessageHandler test #7859
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Context: dotnet#7785 Context: dotnet@7b2e172 7b2e172 added changes to retry the test up to 5 times should https://httpbin.org fail the request (e.g. because of throttling). However, I skipped building the changes locally believing that should anything be wrong, that CI would catch it. And it did, but the failures weren't in the `[Tests]` tab but rather in a stage build log: the `Mono.Android.NET-Tests.csproj` failed to build because the `[Retry]` attribute is not available in `NUnitLite` which this test suite uses. Additionally, `Assert.Warn` isn't supported either and the `DoCompression` method should have been marked `async`. Murphy must be laughing **really** hard now... :) It should work now.
jonpryor
reviewed
Mar 6, 2023
| Assert.Warn ("Unexpected exception thrown"); | ||
| Assert.Warn (ex.ToString ()); | ||
| Console.WriteLine ("Unexpected exception thrown"); | ||
| Console.WriteLine (ex.ToString ()); |
There was a problem hiding this comment.
Should this instead use TestContext.Out.WriteLine()?
There was a problem hiding this comment.
Yes, I think that is the only way it would make it into a log.
There was a problem hiding this comment.
neither TestContext.Out.WriteLine nor TestContext.WriteLine work. It appears that NUnitLite doesn't have that feature either :(
There was a problem hiding this comment.
run-Mono.Android.NET_Tests-Debug.binlog has lines like:
02-28 17:33:35.485 7425 7445 I NUnit : JniValueMarshalerContractTests`1.CreateObjectReferenceArgumentState_DefaultValue
02-28 17:33:35.486 7425 7445 I NUnit : Passed
02-28 17:33:35.486 7425 7445 I NUnit : JniValueMarshalerContractTests`1.CreateReturnValueFromManagedExpression
02-28 17:33:35.494 7425 7445 I DOTNET : # jonp: expected: JniValueMarshaler_String_ContractTests
Ok, for this test Console.WriteLine() will work fine:
https://github.com/xamarin/java.interop/blob/77800dda83c2db4d90b501c00069abc9880caaeb/tests/Java.Interop-Tests/Java.Interop/JniValueMarshalerContractTests.cs#L267
It wouldn't work in our MSBuild tests, but Device Tests w/ NUnitLite this is the only option.
jonathanpeppers
approved these changes
Mar 6, 2023
jpobst
approved these changes
Mar 6, 2023
grendello
added a commit
to grendello/xamarin-android
that referenced
this pull request
Mar 7, 2023
* main: Bump r8 from 4.0.48 to 4.0.52 (dotnet#7858) [tests] Fix recently added AndroidMessageHandler test (dotnet#7859) [build] remove darc dependency for System.IO.Hashing (dotnet#7855) Localized file check-in by OneLocBuild Task (dotnet#7851)
grendello
added a commit
to grendello/xamarin-android
that referenced
this pull request
Mar 9, 2023
* main: [ci] Don't run classic tests on the Windows build/test stage (dotnet#7860) Bump to xamarin/Java.Interop/main@73ebad2 (dotnet#7861) Bump r8 from 4.0.48 to 4.0.52 (dotnet#7858) [tests] Fix recently added AndroidMessageHandler test (dotnet#7859) [build] remove darc dependency for System.IO.Hashing (dotnet#7855) Localized file check-in by OneLocBuild Task (dotnet#7851)
grendello
added a commit
to grendello/xamarin-android
that referenced
this pull request
Mar 9, 2023
* main: [ci] Don't run classic tests on the Windows build/test stage (dotnet#7860) Bump to xamarin/Java.Interop/main@73ebad2 (dotnet#7861) Bump r8 from 4.0.48 to 4.0.52 (dotnet#7858) [tests] Fix recently added AndroidMessageHandler test (dotnet#7859) [build] remove darc dependency for System.IO.Hashing (dotnet#7855) Localized file check-in by OneLocBuild Task (dotnet#7851)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: #7785
Context: 7b2e172
7b2e172 added changes to retry the test up to 5 times should
https://httpbin.org fail the request (e.g. because of throttling).
However, I skipped building the changes locally believing that should
anything be wrong, that CI would catch it. And it did, but the failures
weren't in the
[Tests]tab but rather in a stage build log: theMono.Android.NET-Tests.csprojfailed to build because the[Retry]attribute is not available in
NUnitLitewhich this test suite uses.Additionally,
Assert.Warnisn't supported either and theDoCompressionmethod should have been markedasync.Murphy must be laughing really hard now... :)
It should work now.