[Xamarin.Android.Build.Tests] Fix Repeat Build Tests #2582
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
PR dotnet#2515 highlighted an issue with a few of our repeat build tests. Tests were failing because the `_Sign` target was being skipped. Looking at the build logs, `BuildApk` was being called. As was `_CreateBaseApk`, so what is going on? So it turns out that in `BuildApk` we use the `CopyZipIfChanged` method to move the temp zip file over to the actual one. In this case ... the zip was identical! So the "new" zip was not copied.. and so the `_Sign` task did not need to run. How can the zip be the same? It had an updated file? Well wrong.. we just touched the timestamp. In which case the zip ends up with the exact same CRC values for each file. We use the CRC to detect changes.. so no changes. So it turns out our build system was behaving as expected.. it was just our test was invalid. So the fix here is to update the tests to make sure we do change a file. This means the CRC's will change and the targets will run.
jonathanpeppers
approved these changes
Jan 3, 2019
dellis1972
added a commit
that referenced
this pull request
Jan 22, 2019
PR #2515 highlighted an issue with a few of our repeat build tests. Tests were failing because the `_Sign` target was being skipped. Looking at the build logs, the `BuildApk` target was being called, as was the `_CreateBaseApk` target, so what is going on? It turns out that in the `<BuildApk/>` task we use the `MonoAndroidHelper.CopyIfZipChanged()` method to move the temp zip file over to the actual one. In this case, the zip was identical! Thus the "new" zip was not copied, and the `_Sign` target did not need to run. How can the zip be the same? Didn't it have an updated file? Not quite; we just touched the timestamp. In this case the zip ends up with the exact same CRC values for each file. We use the CRC to detect changes, so no changes were detected. It turns out our build system was behaving as expected, it was just our test was invalid. The fix here is to update the tests to make sure we do change a file. This means the CRC's will change and the targets will run.
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.
PR #2515 highlighted an issue with a few of our
repeat build tests. Tests were failing because
the
_Signtarget was being skipped. Lookingat the build logs,
BuildApkwas being called.As was
_CreateBaseApk, so what is going on?So it turns out that in
BuildApkwe use theCopyZipIfChangedmethod to move the tempzip file over to the actual one. In this case
... the zip was identical! So the "new" zip
was not copied.. and so the
_Signtask didnot need to run.
How can the zip be the same? It had an updated
file? Well wrong.. we just touched the timestamp.
In which case the zip ends up with the exact same
CRC values for each file. We use the CRC to detect
changes.. so no changes. So it turns out our build
system was behaving as expected.. it was just our
test was invalid.
So the fix here is to update the tests to make sure we
do change a file. This means the CRC's will change and
the targets will run.