Skip to content

Conversation

@jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Oct 19, 2017

The end goal here is to enable Windows users to easily run the various
types of tests. Commands have been migrated to MSBuild, keeping make
commands the same for macOS and linux.

Usage

Windows:

msbuild Xamarin.Android.sln /t:RunAllTests
msbuild Xamarin.Android.sln /t:RunNUnitTests
msbuild Xamarin.Android.sln /t:RunJavaInteropTests
msbuild Xamarin.Android.sln /t:RunApkTests

macOS/linux should remain unchanged:

make run-all-tests
make run-nunit-tests
make run-ji-tests
make run-apk-tests

Changes

  • Added a new build-tools/scripts/RunTests.targets
  • Before.Xamarin.Android.sln.targets includes these test targets
  • A new SetEnvironmentVariable task is needed, added to xa-prep-tasks
  • Mono.Android-Tests.csproj needs to remove the <ProjectReference /> android-toolchain
    • Otherwise xa-prep-tasks.dll can become locked on Windows
    • A nested xabuild.exe call will attempt to ovewrite it
  • Update .gitignore for *.rawproto and the generated Xamarin.Android.Common.props

NOTE

Don't merge quite yet, still working on this (hope the build passes). It might be possible to cleanup further with another PR to Java.Interop.

@jonathanpeppers jonathanpeppers added the do-not-merge PR should not be merged. label Oct 19, 2017
@dnfclas
Copy link

dnfclas commented Oct 19, 2017

@jonathanpeppers,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@jonathanpeppers jonathanpeppers force-pushed the run-nunit-targets branch 2 times, most recently from 603929a to ef09e70 Compare October 20, 2017 15:11
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this pull request Oct 20, 2017
This ensures the following values are set on Windows:
- MONO_TRACE_LISTENER
- JAVA_INTEROP_GREF_LOG
- JAVA_INTEROP_LREF_LOG

This also cleans up what needs to be done downstream in xamarin-android:
dotnet/android#949
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this pull request Oct 20, 2017
This ensures the following values are set on Windows:
- MONO_TRACE_LISTENER
- JAVA_INTEROP_GREF_LOG
- JAVA_INTEROP_LREF_LOG

This also cleans up what needs to be done downstream in xamarin-android:
dotnet/android#949
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this pull request Oct 23, 2017
This ensures the following values are set on Windows:
- MONO_TRACE_LISTENER
- JAVA_INTEROP_GREF_LOG
- JAVA_INTEROP_LREF_LOG

This also cleans up what needs to be done downstream in xamarin-android:
dotnet/android#949

This required a new `Java.Interop.BootstrapTasks` project
that adds a `SetEnvironmentVariable` MSBuild task.
This will be useful down the road, for setting "prepare"
steps in this repo for Windows.
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this pull request Oct 23, 2017
This ensures the following values are set on Windows:
- MONO_TRACE_LISTENER
- JAVA_INTEROP_GREF_LOG
- JAVA_INTEROP_LREF_LOG

This also cleans up what needs to be done downstream in xamarin-android:
dotnet/android#949

This required a new `Java.Interop.BootstrapTasks` project
that adds a `SetEnvironmentVariable` MSBuild task.
This will be useful down the road, for setting "prepare"
steps in this repo for Windows.

Also minor updates to `.gitignore` file.
jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this pull request Oct 23, 2017
This ensures the following values are set on Windows:
- MONO_TRACE_LISTENER
- JAVA_INTEROP_GREF_LOG
- JAVA_INTEROP_LREF_LOG

This also cleans up what needs to be done downstream in xamarin-android:
dotnet/android#949

This required a new `Java.Interop.BootstrapTasks` project
that adds a `SetEnvironmentVariable` MSBuild task.
This will be useful down the road, for setting "prepare"
steps in this repo for Windows.

Also minor updates to `.gitignore` file.
# $(call RUN_NUNIT_TEST,filename,log-lref?)
define RUN_NUNIT_TEST
MONO_TRACE_LISTENER=Console.Out \
$(RUNTIME) --runtime=v4.0.0 \
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, this removal will need updating as per PR #965.

<UsingTask AssemblyFile="$(MSBuildThisFileDirectory)..\..\bin\Build$(Configuration)\xa-prep-tasks.dll" TaskName="Xamarin.Android.BuildTools.PrepTasks.SetEnvironmentVariable" />
<PropertyGroup>
<XAIntegratedTests Condition=" '$(HostOS)' == 'Windows' ">False</XAIntegratedTests>
<_Runtime Condition=" '$(RUNTIME)' != '' ">$(RUNTIME)</_Runtime>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should use $(ManagedRuntime), defined in Configuration.props.

<_Runtime Condition=" '$(RUNTIME)' == '' And '$(HostOS)' != 'Windows' ">mono</_Runtime>
<_NUnit>$(_Runtime) packages\NUnit.ConsoleRunner.3.7.0\tools\nunit3-console.exe</_NUnit>
<_Test Condition=" '$(TEST)' != '' ">--test=&quot;$(TEST)&quot;</_Test>
<_XABuild Condition=" '$(HostOS)' == 'Windows' ">$(_TopDir)\bin\$(Configuration)\bin\xabuild.exe</_XABuild>
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't/shouldn't we always just invoke $(_TopDir)\bin\$(Configuration)\bin\xabuild, and let the xabuild script sort it out?

</PropertyGroup>
<ItemGroup>
<_TestAssembly Include="$(_TopDir)\bin\Test$(Configuration)\Xamarin.Android.Build.Tests.dll" Condition=" '$(TestAssembly)' == '' " />
<_TestAssembly Include="$(TestAssembly)" Condition=" '$(TestAssembly)' != '' " />
Copy link
Contributor

Choose a reason for hiding this comment

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

What sets $(TestAssembly), and why do we need to care about it?

<MSBuild
Projects="$(MSBuildThisFileDirectory)TestApks.targets"
Targets="RenameTestCases"
Properties="Configuration=$(Configuration);RenameTestCasesGlob=$(_RenamedTestCases)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this actually work? Properties is a ;-delimited list of properties, and $(_RenamedTestCases) may in turn contain ;s. If that works....wow.

Copy link
Member Author

Choose a reason for hiding this comment

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

This does seem to work, even under xbuild. I will need to read the results of the Jenkins build carefully to make sure it looks the same as older builds.

@jonpryor
Copy link
Contributor

If possible, can we split the Java.Interop bump into a separate PR? It'll make it easier to reason about things if this PR is smaller.

jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this pull request Oct 25, 2017
This ensures the following values are set on Windows:
- MONO_TRACE_LISTENER
- JAVA_INTEROP_GREF_LOG
- JAVA_INTEROP_LREF_LOG

This also cleans up what needs to be done downstream in xamarin-android:
dotnet/android#949

This required a new `Java.Interop.BootstrapTasks` project
that adds a `SetEnvironmentVariable` MSBuild task.
This will be useful down the road, for setting "prepare"
steps in this repo for Windows.

Mono 5.8 also seems to have introduced a slowness in the
PerformanceTests, included a fix to allow more digits in
the time elapsed. This prevents a test failure on Mono 5.8.

Also minor updates to `.gitignore` file.
jonpryor pushed a commit to dotnet/java-interop that referenced this pull request Oct 25, 2017
This ensures the following values are set on Windows:

  - MONO_TRACE_LISTENER
  - JAVA_INTEROP_GREF_LOG
  - JAVA_INTEROP_LREF_LOG

This also cleans up what needs to be done downstream in xamarin-android:
dotnet/android#949

This required a new `Java.Interop.BootstrapTasks` project
that adds a `<SetEnvironmentVariable/>` MSBuild task.
This will be useful down the road, for setting "prepare"
steps in this repo for Windows.

Running `PerformanceTests` also is occasionally, *randomly*, slower
than expected. (The exact reason why is currently unknown.) When it's
"too" slow, an `ArgumentOutOfRangeException` was being thrown from
`FormatFraction()` because the overall width for the time was longer
than what was permitted. Increase the width to allow more digits.

Minor updates to `.gitignore` file.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Oct 25, 2017
Redth pushed a commit to Redth/java.interop that referenced this pull request Oct 26, 2017
This ensures the following values are set on Windows:

  - MONO_TRACE_LISTENER
  - JAVA_INTEROP_GREF_LOG
  - JAVA_INTEROP_LREF_LOG

This also cleans up what needs to be done downstream in xamarin-android:
dotnet/android#949

This required a new `Java.Interop.BootstrapTasks` project
that adds a `<SetEnvironmentVariable/>` MSBuild task.
This will be useful down the road, for setting "prepare"
steps in this repo for Windows.

Running `PerformanceTests` also is occasionally, *randomly*, slower
than expected. (The exact reason why is currently unknown.) When it's
"too" slow, an `ArgumentOutOfRangeException` was being thrown from
`FormatFraction()` because the overall width for the time was longer
than what was permitted. Increase the width to allow more digits.

Minor updates to `.gitignore` file.
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Oct 27, 2017
@jonathanpeppers jonathanpeppers force-pushed the run-nunit-targets branch 3 times, most recently from e5bb3a9 to 8ac0a77 Compare October 31, 2017 03:59
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Nov 1, 2017
Use `bin/$(CONFIGURATION)/bin/xabuild` instead of `tools/xabuild`

This will help figure out what is going on in dotnet#949
@jonathanpeppers jonathanpeppers force-pushed the run-nunit-targets branch 2 times, most recently from 7906ed0 to ecd9956 Compare November 3, 2017 19:25
@jonathanpeppers
Copy link
Member Author

I think this PR is haunted 👻

@jonpryor
Copy link
Contributor

jonpryor commented Nov 5, 2017

The PR is haunted because mono crashed

Native stacktrace:

	0   mono                                0x000000010e233f31 mono_handle_native_crash + 257
	1   mono                                0x000000010e29d6e6 altstack_handle_and_restore + 70
	2   mono                                0x000000010e3d1744 sgen_client_par_object_get_size + 4
	3   mono                                0x000000010e3d1c0b pin_objects_in_nursery + 683
	4   mono                                0x000000010e3ce1d0 collect_nursery + 512
	5   mono                                0x000000010e3cdc05 sgen_perform_collection_inner + 533
	6   mono                                0x000000010e3d63bd sgen_los_alloc_large_inner + 93
	7   mono                                0x000000010e3c319f sgen_alloc_obj_nolock + 95
	8   mono                                0x000000010e3c07f7 mono_gc_alloc_vector + 119
	9   ???                                 0x000000010fcbc775 0x0 + 4559980405
	10  mscorlib.dll.dylib                  0x0000000114262c1c System_IO_BinaryReader_ReadBytes_int + 124

Rebase on b3e9820?

@jonathanpeppers jonathanpeppers force-pushed the run-nunit-targets branch 2 times, most recently from adb1f1c to ed4c7cc Compare November 6, 2017 15:53
@jonathanpeppers jonathanpeppers removed the do-not-merge PR should not be merged. label Nov 7, 2017
The end goal here is to enable Windows users to easily run the various
types of tests. Commands have been migrated to MSBuild, keeping `make`
commands the same for macOS and linux.

~~ Usage ~~

Windows:
```
msbuild Xamarin.Android.sln /t:RunAllTests
msbuild Xamarin.Android.sln /t:RunNUnitTests
msbuild Xamarin.Android.sln /t:RunJavaInteropTests
msbuild Xamarin.Android.sln /t:RunApkTests
```

macOS/linux should remain unchanged:
```
make run-all-tests
make run-nunit-tests
make run-ji-tests
make run-apk-tests
```

~~ Changes ~~

- Added a new build-tools/scripts/RunTests.targets
- Before.Xamarin.Android.sln.targets includes these test targets
- A new `SetEnvironmentVariable` task is needed, added to xa-prep-tasks
- Ported any environment variables set in Makefile to MSBuild
- Mono.Android-Tests.csproj needs to remove the `<ProjectReference />` xa-prep-tasks
  - Otherwise xa-prep-tasks.dll can become locked on Windows
  - A nested xabuild.exe call will attempt to ovewrite it
- Update .gitignore for *.rawproto and the generated Xamarin.Android.Common.props
@jonpryor jonpryor merged commit cdf3bcc into dotnet:master Nov 8, 2017
jonpryor pushed a commit that referenced this pull request Feb 11, 2022
Fixes: dotnet/java-interop#945

Changes: dotnet/java-interop@32635fd...7dc270d

  * dotnet/java-interop@7dc270db: [build] set $(ProduceReferenceAssemblyInOutDir)=True (#949)
  * dotnet/java-interop@f91b0770: [Xamarin.Android.Tools.Bytecode] Improve Kotlin metadata matching (#946)
@jonathanpeppers jonathanpeppers deleted the run-nunit-targets branch April 26, 2023 15:31
@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants