Skip to content

Conversation

@radekdoulik
Copy link
Member

We see these errors in our jenkins builds:

/Users/builder/jenkins/workspace/xamarin-android-pr-builder-release/xamarin-android/build-tools/scripts/TestApks.targets(217,5): error : Input file 'logcat-Release-Aot-Mono.Android_TestsMultiDex.txt' doesn't exist. [/Users/builder/jenkins/workspace/xamarin-android-pr-builder-release/xamarin-android/tests/RunApkTests.targets]
  Build continuing because "ContinueOnError" on the task "ProcessLogcatTiming" is set to "ErrorAndContinue".

Turns out, that the reason for that is, that the
Mono.Android_TestsMultiDex test is not run, because no
RunInstrumentationTests nor RunUITests is used as their condition
clauses are resolved as false.

This is fixed by adding TestApkInstrumentation (and
TestApkPermission plus TestApkPermission) item groups. This makes
the test run thru the RunInstrumentationTests task.

@radekdoulik radekdoulik added the do-not-merge PR should not be merged. label Feb 8, 2019
@radekdoulik radekdoulik requested a review from jonpryor as a code owner February 8, 2019 16:33
@radekdoulik
Copy link
Member Author

radekdoulik commented Feb 8, 2019

This is to test for now, that it works on Jenkins.

I want to make sure, the mentioned error will fail the build, unlike the current situations, where the build is green, even when the test didn't run and the ProcessLogcatOutput failed. => do-not-merge label.

That will be addressed in another PR.

@jonpryor
Copy link
Contributor

jonpryor commented Feb 8, 2019

Good $DEITY, how long has multidex been broken?

@radekdoulik
Copy link
Member Author

Hard to tell without the test running for long time ... :-)

@dellis1972 do you have an idea, what might be breaking these tests?

@dellis1972
Copy link
Contributor

The MultiDex test is missing some additions that this commit introduced f356220.

namely

  <ItemGroup>
	    <AndroidAsset Include="Assets\asset1.txt" />
	    <AndroidAsset Include="Assets\subfolder\asset2.txt" />
 </ItemGroup>

These are not in the same folder, so we will need to add the correct pathing and Logical Names to get that to share the asset*.txt files. Like we do with https://github.com/xamarin/xamarin-android/blob/master/tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj#L74

@dellis1972
Copy link
Contributor

@radekdoulik can you rebase this on master.

We see these errors in our jenkins builds:

    /Users/builder/jenkins/workspace/xamarin-android-pr-builder-release/xamarin-android/build-tools/scripts/TestApks.targets(217,5): error : Input file 'logcat-Release-Aot-Mono.Android_TestsMultiDex.txt' doesn't exist. [/Users/builder/jenkins/workspace/xamarin-android-pr-builder-release/xamarin-android/tests/RunApkTests.targets]
      Build continuing because "ContinueOnError" on the task "ProcessLogcatTiming" is set to "ErrorAndContinue".

Turns out, that the reason for that is, that the
`Mono.Android_TestsMultiDex` test is not run, because no
`RunInstrumentationTests` nor `RunUITests` is used as their condition
clauses are resolved as **false**.

This is fixed by adding `TestApkInstrumentation` (and
`TestApkPermission` plus `TestApkPermission`) item groups. This makes
the test run thru the `RunInstrumentationTests` task.
@radekdoulik radekdoulik force-pushed the pr-run-Mono.Android_TestsMultiDex-test branch from 2fcbef1 to 24100b2 Compare February 11, 2019 18:24
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this pull request Feb 14, 2019
It is usually hard to notice the problem, when logcat processing fails
because of the logcat file missing. Thus it will make it easier to
find, when we create `TestResults` file with an error, similar to
`RenameTestCases` task.

Also added a conditional `Error` task after `RunInstrumentationTests`
and `RunUITests` tasks, to catch insufficiently specified
tests. Context: dotnet#2715
radekdoulik added a commit to radekdoulik/xamarin-android that referenced this pull request Feb 14, 2019
It is usually hard to notice the problem, when logcat processing fails
because of the logcat file missing. Thus it will make it easier to
find, when we create `TestResults` file with an error, similar to
`RenameTestCases` task.

Also added a conditional `Error` task after `RunInstrumentationTests`
and `RunUITests` tasks, to catch insufficiently specified
tests. Context: dotnet#2715
jonpryor pushed a commit that referenced this pull request Feb 15, 2019
Errors from `<ProcessLogcatTiming/>` were being ignored.

Consider [master build #1518][0]: the build is green, which *should*
mean that everything built, all tests *ran*, and all tests *pass*.
However, the build log contains -- and has contained for quite some
time! -- an error message:

	build-tools/scripts/TestApks.targets(217,5): error : Input file 'logcat-Release-Aot-Mono.Android_TestsMultiDex.txt' doesn't exist.

This has also been mentioned in commit 13a6925, and this error was
added in commit 5eae6e5 so that we wouldn't miss these errors.

Unfortunately, we've *still* been "missing"/overlooking these errors,
in large part because these errors aren't surfaced.

Update `<ProcessLogcatTiming/>` to follow `<RenameTestCases/>` and
generate an NUnit XML file containing an error message, which Jenkins
can then display alongside any other unit test errors; see also
commits 0ec00e1, 1db436c, and c38c58e.

This way, when `<ProcessLogcatTiming/>` fails, the failure will be
visible, which in turn will help us to fix the failure.  We can't fix
what we don't know about.

Related: PR #2715 will fix the reported error about
`logcat-Release-Aot-Mono.Android_TestsMultiDex.txt` not existing.
We didn't want to merge PR #2715 until improving
`<ProcessLogcatTiming/>` (this commit), for verification purposes.

[0]: https://jenkins.mono-project.com/view/Xamarin.Android/job/xamarin-android/1518
@radekdoulik
Copy link
Member Author

build

@jonpryor jonpryor merged commit 5676d26 into dotnet:master Feb 15, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

do-not-merge PR should not be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants