Skip to content

Conversation

jonpryor
Copy link
Contributor

@jonpryor jonpryor commented Sep 19, 2017

What do we want? (with apologies to 48e3fc2)
MOAR Unit tests!

Specifically, we want to run the BCL unit tests which mono generates:

    $ cd external/mono/mcs/class/corlib
    $ make PROFILE=monodroid test
    # creates `monodroid_corlib_test.dll`

Creation of monodroid_*_test.dll assemblies and the
make PROFILE=monodroid test target is a relatively recent
development, for which I need to buy the #runtime team some beers.

In terms of mono-runtimes.targets, we can build all of the BCL
unit test assemblies with:

    $ cd external/mono/mcs/class
    $ make -i do-test PROFILE=monodroid

Now that we can create them, how do we use them? That's the trickier
bit: they need to be built within mono, as part of the existing BCL
build process. This in turn means that the BCL unit test assemblies
need to be distributed as part of the mono bundle, as we don't want to
rebuild the mono repo "from scratch" just for the unit tests.

Update build-tools/mono-runtimes/ProfileAssemblies.projitems to
include a new @(MonoTestAssembly) item group which contains all of
the BCL unit test assemblies and related files which should be
included into bundle-*.zip. Additionally, add
ProfileAssemblies.projitems to @(VersionFile) witihin
bundle-path.targets, so that if anything within
ProfileAssemblies.projitems changes, we rebuild the bundle.

Once we have the BCL unit test assemblies, and their dependencies,
we need to run them. The new Xamarin.Android.Bcl-Tests.csproj
project is a Xamarin.Android application project which will execute
the unit tests.

There's just one small problem: Xamarin.Android apps want to use
Xamarin.Android.NUnitLite.dll. The BCL unit test assemblies instead
build against their own nunitlite.dll, which has no Xamarin.Android
integration or support. How do we use the new test assemblies?

Force a fix by using remap-assembly-ref to "rename" the
nunitlite assembly reference to Xamarin.Android.NUnitLite.dll.
This cannot be done as part of the mono-runtimes.mdproj build, as
Xamarin.Android.NUnitLite.dll won't yet exist. Instead, remap the
assemblies within Xamarin.Android.Bcl-Tests.targets, and distribute
the remapped assemblies with the application.

Finally, address one other "small" problem: not all of the unit tests
pass! Some of these are for reasons we don't know, and others will
require changes to mono.

Update Xamarin.Android.NUnitLite to allow filtering of tests:

    namespace Xamarin.Android.NUnitLite {

            partial class TestSuiteActivity {
                    public  ITestFilter   Filter          {get; set;}

                    public  virtual void  UpdateFilter ();
            }

            partial class TestSuiteInstrumentation {
                    public  ITestFilter   Filter          {get; set;}

                    public  virtual void  UpdateFilter ();
            }
    }

TestSuiteActivity.UpdateFilter() is called by
TestSuiteActivity.OnCreate(), after GetIncludedCategories() and
GetExcludedCategories() are called, to allow subclasses to alter the
ITestFilter which is used to determine which tests are executed.

TestSuiteInstrumentation.UpdateFilter() is called by
TestSuiteInstrumentation.OnStart(), after
GetIncludedCategories() and GetExcludedCategories() are called, to
allow subclasses to alter the ITestFilter which is used to determine
which tests are executed.

Xamarin.Android.Bcl_Tests overrides both of these and updates the
Filter property so that "known failing" tests are excluded. This
allows us to skip failing tests, giving us time to properly fix them
in time while allowing the rest of this PR to be merged.

The skipped tests include:

  • MonoTests.System.Reflection.AssemblyTest.GetReferencedAssemblies
  • MonoTests.System.ServiceModel.Description.WebInvokeAttributeTest.RejectTwoParametersWhenNotWrapped

@jonpryor
Copy link
Contributor Author

Good news! We don't need to try to migrate over the spaghetti code that is monodroid/tests/bcl-test.

Bad news! I haven't tried running all of these tests locally, just the corlib tests, and they didn't all pass...

$(BuildDependsOn)
</BuildDependsOn>
</PropertyGroup>
</Project> No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Not sure, whether we care about newlines at the end of file.

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 don't. Some of my editors add them, some don't.

What's really amusing is that some tools check for newline at end of file...even though most tools which create this file -- Visual Studio! -- never put a newline at EOF. :-)

<PropertyGroup>
<_Assemblies>@(MonoTestAssembly->'"%(Identity)"', ', ')</_Assemblies>
</PropertyGroup>
<ReplaceFileContents
Copy link
Member

Choose a reason for hiding this comment

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

ReplaceFileContents task tries to delete the destination file even when it doesn't exists and thus failing with an unhandled exception. I think we need to add check with File.Exists at https://github.com/xamarin/xamarin-android/blob/master/build-tools/xa-prep-tasks/Xamarin.Android.BuildTools.PrepTasks/ReplaceFileContents.cs#L32

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@radekdoulik: File.Delete() doesn't throw if a file doesn't exist:

If the file to be deleted does not exist, no exception is thrown.

The build failure is because the obj/Debug directory doesn't exist, and thus it can't create the file obj/Debug/App.cs.

Copy link
Member

Choose a reason for hiding this comment

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

Yup, I wasn't exact. It was caused by the missing directory (DirectoryNotFoundException). It was thrown by the File.Delete though.

01:51:55 				Error executing task ReplaceFileContents: System.IO.DirectoryNotFoundException: Could not find a part of the path "obj/Debug/App.cs".
01:51:55 				  at System.IO.File.Delete (System.String path) [0x00074] in /Users/builder/data/lanes/5263/mono-mac-sdk/external/bockbuild/builds/mono-x64/mcs/class/corlib/System.IO/File.cs:179 
01:51:55 				  at Xamarin.Android.BuildTools.PrepTasks.ReplaceFileContents.Execute () [0x000d9] in /Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/build-tools/xa-prep-tasks/Xamarin.Android.BuildTools.PrepTasks/ReplaceFileContents.cs:32```

@jonpryor jonpryor force-pushed the jonp-bcl-test branch 3 times, most recently from c299f0b to c8f1820 Compare September 20, 2017 02:31
@jonpryor
Copy link
Contributor Author

Good news! The BCL tests are now executing!

Bad news: 384 of those tests fail

We can't merge 384 failures. :-)

That leaves the question of how to proceed. Most of those failures are because the tests are referencing files which don't exist within the Xamarin.Android app. Xamarin.Android likewise doesn't support @(Content), so there's not a straightforward way to get those files either.

I see three plausible approaches:

  1. We (somehow) disable all the tests/test fixtures which require external files. All 384+ of them. This should be somewhat reasonably straightforward, if annoying.

  2. We fix the tests! (Timeframe unknown!)

  3. We (somehow) package and distribute the required files, and e.g. extract those files on first launch. This could allow the unit tests to execute ~normally.

@jonpryor
Copy link
Contributor Author

For "great fun," the macOS+msbuild failure:

error MSB3030: Could not copy the file "/Users/builder/jenkins/workspace/xamarin-android-msbuild-pr-builder/external/mono/mcs/class/System.XML/Test/**/*.*" because it was not found.

@radical thinks that this might be due to/a variation on: dotnet/msbuild#406

I believe that this is caused by:

<_BclTestContent Include="@(MonoTestAssembly->'$(MonoSourceFullPath)\mcs\class\%(SourcePath)\Test\**\*.*')" />

I thus suspect that the fix will be to avoid using @(MonoTestAssembly), and instead hardcode the list of directories. :-(

@jonpryor
Copy link
Contributor Author

macOS+xbuild is down to 49 failures (from 384!).

I have a local patch which drops that to 9. Progress!

@jonpryor jonpryor force-pushed the jonp-bcl-test branch 3 times, most recently from 3297914 to 0bbeaa3 Compare October 2, 2017 18:57
@jonpryor
Copy link
Contributor Author

jonpryor commented Oct 2, 2017

This earlier comment:

I thus suspect that the fix will be to avoid using @(MonoTestAssembly), and instead hardcode the list of directories.

was fortunately wrong. The fix to appease msbuild is:

<_BclTestContent      Include="$(MonoSourceFullPath)\mcs\class\%(MonoTestAssembly.SourcePath)\Test\**\*.*" />

Files="@(_RuntimeSource);@(_ProfilerSource);@(_MonoPosixHelperSource);@(_BclProfileItems);@(_MonoBtlsSource)"
/>
<Exec
Command="make $(MakeConcurrency) -i do-test PROFILE=monodroid"
Copy link
Member

Choose a reason for hiding this comment

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

I'd use test instead of do-test, it's the more canonical target.

Copy link
Member

Choose a reason for hiding this comment

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

also do you really need -i? which errors do you get?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akoeplinger: Yes, I really need -i:

^[[Dmake[6]: *** No rule to make target `Test/../../../../external/api-doc-tools/monodoc/Test/Monodoc.Ecma/EcmaUrlTests.cs', needed by `net_4_x_monodoc_test.dll'.  Stop.

Alexander Kyte suggested make do-trst, and Zoltan mentioned that Mono Jenkins uses -i.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see now you're compiling from mcs/class so you compile all assemblies even those that don't apply to you (like monodoc in this case, though still weird that it can't find the file in the submodule).

Please switch over to running make -i test in $(MonoSourceFullPath)\runtime, this will only compile the tests of assemblies which are included in the monodroid profile.

Ignoring errors shouldn't be required but it is right now, because the following test assemblies don't build in your profile (something to look at):

make[3]: [monodroid_System.Security_test.dll] Error 1 (ignored)
make[3]: [monodroid_System.IdentityModel_test.dll] Error 1 (ignored)
make[3]: [monodroid_System.ServiceModel_test.dll] Error 1 (ignored)
make[3]: [monodroid_System.Web.Services_test.dll] Error 1 (ignored)
make[3]: [monodroid_System.ComponentModel.DataAnnotations_test.dll] Error 1 (ignored)
make[3]: [monodroid_Mono.Posix_test.dll] Error 1 (ignored)

static string[] ExcludeTestNames = new string[]{
"MonoTests.System.Reflection.AssemblyTest.GetReferencedAssemblies",
"MonoTests.System.Resources.ResourceManagerTest.TestSatellites",
"MonoTests.System.ServiceModel.Description.WebInvokeAttributeTest.RejectTwoParametersWhenNotWrapped",
Copy link
Member

Choose a reason for hiding this comment

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

is there already a bugzilla bug for investigating this? :)

@jonpryor jonpryor force-pushed the jonp-bcl-test branch 2 times, most recently from a096fcd to c397d3b Compare October 3, 2017 01:33
Files="@(_RuntimeSource);@(_ProfilerSource);@(_MonoPosixHelperSource);@(_BclProfileItems);@(_MonoBtlsSource)"
/>
<Exec
Command="make $(MakeConcurrency) -i do-test PROFILE=monodroid"
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see now you're compiling from mcs/class so you compile all assemblies even those that don't apply to you (like monodoc in this case, though still weird that it can't find the file in the submodule).

Please switch over to running make -i test in $(MonoSourceFullPath)\runtime, this will only compile the tests of assemblies which are included in the monodroid profile.

Ignoring errors shouldn't be required but it is right now, because the following test assemblies don't build in your profile (something to look at):

make[3]: [monodroid_System.Security_test.dll] Error 1 (ignored)
make[3]: [monodroid_System.IdentityModel_test.dll] Error 1 (ignored)
make[3]: [monodroid_System.ServiceModel_test.dll] Error 1 (ignored)
make[3]: [monodroid_System.Web.Services_test.dll] Error 1 (ignored)
make[3]: [monodroid_System.ComponentModel.DataAnnotations_test.dll] Error 1 (ignored)
make[3]: [monodroid_Mono.Posix_test.dll] Error 1 (ignored)

What do we want? (with apologies to 48e3fc2)
**MOAR** Unit tests!

Specifically, we want to run the BCL unit tests which mono generates:

	$ cd external/mono/mcs/class/corlib
	$ make PROFILE=monodroid test
	# creates `monodroid_corlib_test.dll`

Creation of `monodroid_*_test.dll` assemblies and the
`make PROFILE=monodroid test` target is a relatively recent
development, for which I need to buy the #runtime team some beers.

In terms of `mono-runtimes.targets`, we can build *all* of the BCL
unit test assemblies with:

	$ cd external/mono/mcs/class
	$ make -i do-test PROFILE=monodroid

Now that we can create them, how do we *use* them? That's the trickier
bit: they need to be built within mono, as part of the existing BCL
build process. This in turn means that the BCL unit test assemblies
need to be distributed as part of the mono bundle, as we don't want to
rebuild the mono repo "from scratch" just for the unit tests.

Update `build-tools/mono-runtimes/ProfileAssemblies.projitems` to
include a new `@(MonoTestAssembly)` item group which contains all of
the BCL unit test assemblies and related files which should be
included into `bundle-*.zip`. Additionally, add
`ProfileAssemblies.projitems` to `@(VersionFile)` witihin
`bundle-path.targets`, so that if anything within
`ProfileAssemblies.projitems` changes, we rebuild the bundle.

Once we *have* the BCL unit test assemblies, and their dependencies,
we need to *run* them. The new `Xamarin.Android.Bcl-Tests.csproj`
project is a Xamarin.Android application project which will execute
the unit tests.

There's just one small problem: Xamarin.Android apps want to use
`Xamarin.Android.NUnitLite.dll`. The BCL unit test assemblies instead
build against their own `nunitlite.dll`, which has no Xamarin.Android
integration or support. How do we use the new test assemblies?

*Force* a fix by using `remap-assembly-ref` to "rename" the
`nunitlite` assembly reference to `Xamarin.Android.NUnitLite.dll`.
This *cannot* be done as part of the `mono-runtimes.mdproj` build, as
`Xamarin.Android.NUnitLite.dll` won't yet exist. Instead, remap the
assemblies within `Xamarin.Android.Bcl-Tests.targets`, and distribute
the remapped assemblies with the application.

Finally, address one other "small" problem: not all of the unit tests
pass! Some of these are for reasons we don't know, and others will
require changes to `mono`.

Update `Xamarin.Android.NUnitLite` to allow *filtering* of tests:

	namespace Xamarin.Android.NUnitLite {

	  partial class TestSuiteActivity {
	    public  ITestFilter   Filter          {get; set;}

	    public  virtual void  UpdateFilter ();
	  }

	  partial class TestSuiteInstrumentation {
	    public  ITestFilter   Filter          {get; set;}

	    public  virtual void  UpdateFilter ();
	  }
	}

`TestSuiteActivity.UpdateFilter()` is called by
`TestSuiteActivity.OnCreate()`, *after* `GetIncludedCategories()` and
`GetExcludedCategories()` are called, to allow subclasses to alter the
`ITestFilter` which is used to determine which tests are executed.

`TestSuiteInstrumentation.UpdateFilter()` is called by
`TestSuiteInstrumentation.OnStart()`, *after*
`GetIncludedCategories()` and `GetExcludedCategories()` are called, to
allow subclasses to alter the `ITestFilter` which is used to determine
which tests are executed.

`Xamarin.Android.Bcl_Tests` overrides both of these and updates the
`Filter` property so that "known failing" tests are excluded. This
allows us to skip failing tests, giving us time to properly fix them
in time while allowing the rest of this PR to be merged.

The skipped tests include:

  * MonoTests.System.Reflection.AssemblyTest.GetReferencedAssemblies
  * MonoTests.System.ServiceModel.Description.WebInvokeAttributeTest.RejectTwoParametersWhenNotWrapped
@grendello grendello merged commit e9daf5e into dotnet:master Oct 3, 2017
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Aug 24, 2021
Changes: http://github.com/xamarin/Java.Interop/compare/ff2714200107fb616828b9d1013f69605791d2ba...8898bc1402953fdf3a1e1066dd2542fc1818aadf

  * dotnet/java-interop@8898bc14: [build] Import $(MSBuildProjectDirectory).override.props (dotnet#872)
  * dotnet/java-interop@d16b1e56: [build] Use GitInfo to generate $(Version) (dotnet#865)
  * dotnet/java-interop@2eb9ff26: [build] .NET 6 P7 Support (dotnet#869)

Set `$(XamarinAndroidToolsDirectory)` for `external/Java.Interop`
so that the Java.Interop build uses `external/xamarin-android-tools`.
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Aug 25, 2021
Changes: http://github.com/xamarin/Java.Interop/compare/ff2714200107fb616828b9d1013f69605791d2ba...9a878f211c86bdd37cb71c5c461f85a0c1a1480d

  * dotnet/java-interop@9a878f21: [build] Properly implement "parent directory.override.props" (dotnet#300) (dotnet#873)
  * dotnet/java-interop@0c5d454c: [Java.Interop] fix .NET 6 linker warnings (dotnet#870)
  * dotnet/java-interop@8898bc14: [build] Import $(MSBuildProjectDirectory).override.props (dotnet#872)
  * dotnet/java-interop@d16b1e56: [build] Use GitInfo to generate $(Version) (dotnet#865)
  * dotnet/java-interop@2eb9ff26: [build] .NET 6 P7 Support (dotnet#869)

Set `$(XamarinAndroidToolsDirectory)` for `external/Java.Interop`
so that the Java.Interop build uses `external/xamarin-android-tools`.
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Aug 26, 2021
Changes: http://github.com/xamarin/Java.Interop/compare/ff2714200107fb616828b9d1013f69605791d2ba...b7982e423952a4c4afe0c33f36ac80eed7fa57a2

  * dotnet/java-interop@b7982e42: Revert "[build] Use GitInfo to generate $(Version) (dotnet#865)" (dotnet#874)
  * dotnet/java-interop@9a878f21: [build] Properly implement "parent directory.override.props" (dotnet#300) (dotnet#873)
  * dotnet/java-interop@0c5d454c: [Java.Interop] fix .NET 6 linker warnings (dotnet#870)
  * dotnet/java-interop@8898bc14: [build] Import $(MSBuildProjectDirectory).override.props (dotnet#872)
  * dotnet/java-interop@d16b1e56: [build] Use GitInfo to generate $(Version) (dotnet#865)
  * dotnet/java-interop@2eb9ff26: [build] .NET 6 P7 Support (dotnet#869)

Set `$(XamarinAndroidToolsDirectory)` for `external/Java.Interop`
so that the Java.Interop build uses `external/xamarin-android-tools`.
jonpryor added a commit that referenced this pull request Aug 26, 2021
Changes: http://github.com/xamarin/Java.Interop/compare/ff2714200107fb616828b9d1013f69605791d2ba...b7982e423952a4c4afe0c33f36ac80eed7fa57a2

  * dotnet/java-interop@b7982e42: Revert "[build] Use GitInfo to generate $(Version) (#865)" (#874)
  * dotnet/java-interop@9a878f21: [build] Properly implement "parent directory.override.props" (#300) (#873)
  * dotnet/java-interop@0c5d454c: [Java.Interop] fix .NET 6 linker warnings (#870)
  * dotnet/java-interop@8898bc14: [build] Import $(MSBuildProjectDirectory).override.props (#872)
  * dotnet/java-interop@d16b1e56: [build] Use GitInfo to generate $(Version) (#865)
  * dotnet/java-interop@2eb9ff26: [build] .NET 6 P7 Support (#869)

Set `$(XamarinAndroidToolsDirectory)` for `external/Java.Interop`
so that the Java.Interop build uses `external/xamarin-android-tools`.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 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.

5 participants