Skip to content

Conversation

@jonpryor
Copy link
Contributor

Context: dotnet/android#2004
Context: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/3656/testReport/junit/Xamarin.Android.Build.Tests/ResolveSdksTaskTests/ResolveSdkTiming___Debug/

Before commit 7553056, the only validation performed on the
AndroidSdkInfo's javaSdkPath parameter was "does
{javaSdkPath}/bin/jarsigner exist?

This is the world the xamarin-android unit tests assumed, as it
would create {javaSdkPath}/bin/jarsigner and
{javaSdkPath}/bin/javac.bash (among others), but not
{javaSdkPath/bin/javac.

Then came commit 7553056, which altered
AndroidSdkUnix.ValidateJavaSdkLocation() semantics so that it now
required the presence of {javaSdkPath}/bin/javac.

This changed caused the xamarin-android unit tests to fail:

JavaSdkPath should be /Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ResolveSdkTiming/jdk
Expected string length 63 but was 115. Strings differ at index 1.
Expected: "/Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home"
But was:  "/Users/builder/jenkins/workspace/xamarin-android-pr-builder/x..."
------------^

The "expected" and "actual" values are reversed; the actual problem is
that the javaSdkPath value provided to the AndroidSdkInfo
constructor --
/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ResolveSdkTiming/jdk
-- is "ignored", because it doesn't pass validation,
prompting AndroidSdkInfo to call AndroidSdkUnix.GetJavaSdkPath(),
which returns a "system" location,
/Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home.

As such, the original javaSdkPath value is not preserved and
returned by the AndroidSdkInfo.JavaSdkPath property.

Fix this by removing the AndroidSdkUnix.ValidateJavaSdkLocation()
method: it doesn't appear to actually be useful in this instance, as
the base AndroidSdkBase.ValidateJavaSdkLocation() already checks
for the presence of {javaSdkPath}/bin/jarsigner.

Context: dotnet/android#2004
Context: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/3656/testReport/junit/Xamarin.Android.Build.Tests/ResolveSdksTaskTests/ResolveSdkTiming___Debug/

Before commit 7553056, the only validation performed on the
`AndroidSdkInfo`'s `javaSdkPath` parameter was "does
`{javaSdkPath}/bin/jarsigner` exist?

This is the world the [xamarin-android unit tests assumed][0], as it
would create `{javaSdkPath}/bin/jarsigner` and
`{javaSdkPath}/bin/javac.bash` (among others), but *not*
`{javaSdkPath/bin/javac`.

Then came commit 7553056, which altered
`AndroidSdkUnix.ValidateJavaSdkLocation()` semantics so that it now
*required* the presence of `{javaSdkPath}/bin/javac`.

This changed caused the xamarin-android unit tests to fail:

	JavaSdkPath should be /Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ResolveSdkTiming/jdk
	Expected string length 63 but was 115. Strings differ at index 1.
	Expected: "/Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home"
	But was:  "/Users/builder/jenkins/workspace/xamarin-android-pr-builder/x..."
	------------^

The "expected" and "actual" values are reversed; the actual problem is
that the `javaSdkPath` value provided to the `AndroidSdkInfo`
constructor --
`/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ResolveSdkTiming/jdk`
-- is "ignored", because it doesn't pass validation,
prompting `AndroidSdkInfo` to call `AndroidSdkUnix.GetJavaSdkPath()`,
which returns a "system" location,
`/Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home`.

As such, the original `javaSdkPath` value is not preserved and
returned by the `AndroidSdkInfo.JavaSdkPath` property.

Fix this by *removing* the `AndroidSdkUnix.ValidateJavaSdkLocation()`
method: it doesn't appear to actually be useful in this instance, as
the base `AndroidSdkBase.ValidateJavaSdkLocation()`  already checks
for the presence of `{javaSdkPath}/bin/jarsigner`.

[0]: https://github.com/xamarin/xamarin-android/blob/997b8904889d371610026c1802a8e3403d7d2c96/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs#L220-L235
@jonpryor jonpryor requested a review from bratsche July 27, 2018 19:35
@jonpryor jonpryor merged commit 07c4c2b into dotnet:master Jul 27, 2018
jonpryor added a commit that referenced this pull request Aug 2, 2018
Context: dotnet/android#2004
Context: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/3656/testReport/junit/Xamarin.Android.Build.Tests/ResolveSdksTaskTests/ResolveSdkTiming___Debug/

Before commit 7553056, the only validation performed on the
`AndroidSdkInfo`'s `javaSdkPath` parameter was "does
`{javaSdkPath}/bin/jarsigner` exist?

This is the world the [xamarin-android unit tests assumed][0], as it
would create `{javaSdkPath}/bin/jarsigner` and
`{javaSdkPath}/bin/javac.bash` (among others), but *not*
`{javaSdkPath/bin/javac`.

Then came commit 7553056, which altered
`AndroidSdkUnix.ValidateJavaSdkLocation()` semantics so that it now
*required* the presence of `{javaSdkPath}/bin/javac`.

This changed caused the xamarin-android unit tests to fail:

	JavaSdkPath should be /Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ResolveSdkTiming/jdk
	Expected string length 63 but was 115. Strings differ at index 1.
	Expected: "/Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home"
	But was:  "/Users/builder/jenkins/workspace/xamarin-android-pr-builder/x..."
	------------^

The "expected" and "actual" values are reversed; the actual problem is
that the `javaSdkPath` value provided to the `AndroidSdkInfo`
constructor --
`/Users/builder/jenkins/workspace/xamarin-android-pr-builder/xamarin-android/bin/TestDebug/temp/ResolveSdkTiming/jdk`
-- is "ignored", because it doesn't pass validation,
prompting `AndroidSdkInfo` to call `AndroidSdkUnix.GetJavaSdkPath()`,
which returns a "system" location,
`/Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home`.

As such, the original `javaSdkPath` value is not preserved and
returned by the `AndroidSdkInfo.JavaSdkPath` property.

Fix this by *removing* the `AndroidSdkUnix.ValidateJavaSdkLocation()`
method: it doesn't appear to actually be useful in this instance, as
the base `AndroidSdkBase.ValidateJavaSdkLocation()`  already checks
for the presence of `{javaSdkPath}/bin/jarsigner`.

[0]: https://github.com/xamarin/xamarin-android/blob/997b8904889d371610026c1802a8e3403d7d2c96/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs#L220-L235
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants