[tests] Fix issue in ProjectBuilder.Cleanup #1005
Conversation
|
@jonathanpeppers, |
|
@dellis1972 let me know if you think there is something else we should fix here instead of changing this test. I wasn't seeing what would make |
| builder.Cleanup (); | ||
| proj.TargetFrameworkVersion = "v4.1"; | ||
| proj.AndroidManifest = string.Format (TargetSdkManifest, "16"); | ||
| proj.OtherBuildItems [0].Timestamp = null; |
There was a problem hiding this comment.
we should probably put this code in the project.AndroidManifest property setter (or something like it) so that if the property is updated it will do this.
jonathanpeppers
force-pushed
the
windows-tests-timestamps
branch
from
ee95311 to
e5185fc
Compare
|
I still don't understand why the timestamp matters. Doesn't NTFS have better file time precision than macOS? This seems confusing. :-( |
|
@jonpryor the original test I was fixing changes the string contents of the manifest throughout the test. There is some logic that is preventing the file from being rewritten--even if its contents changed. For some reason, this is only happening on Windows. |
...and that's what confuses me. What is the "some logic"? Why does this only happen on Windows? It feels like we don't actually know what's going on, it's just that clearing the Timestamp property papers over the issue enough to make it work. |
|
The problem is somewhere in this lot https://github.com/xamarin/xamarin-android/blob/master/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/XamarinProject.cs#L317 I would guess |
This method was not using the full path to the project directory, and so it wasn't actually deleting anything. This caused test failures on Windows in tests such as `ManifestTest.Bug12935`. This particular test was modifying the contents of `AndroidManifest.xml`, and checking the results of the build afterward. Since the project files were not deleted, changes to the `AndroidManifest.xml` were not getting written to disk during the test.
jonathanpeppers
force-pushed
the
windows-tests-timestamps
branch
from
e5185fc to
0ac36cd
Compare
jonathanpeppers
changed the title
|
|
||
| var item = OtherBuildItems?.FirstOrDefault (i => i.Include() == "Properties\\AndroidManifest.xml"); | ||
| if (item != null) { | ||
| item.Timestamp = null; |
There was a problem hiding this comment.
we can get away with using DateTime.UtcNow here I think
There was a problem hiding this comment.
Actually looking at the code.. builder.Cleanup should be deleting the project directory between these builds. Perhaps its that which is not working..
|
@jonathanpeppers looks good. I'll merge once we get a green build |
4 participants
This method was not using the full path to the project directory, and so
it wasn't actually deleting anything. This caused test failures on
Windows in tests such as
ManifestTest.Bug12935. This particular testwas modifying the contents of
AndroidManifest.xml, and checking theresults of the build afterward. Since the project files were not
deleted, changes to the
AndroidManifest.xmlwere not getting writtento disk during the test.