[tests] added forms test #818

radekdoulik · 2017-09-05T11:59:40Z

to measure UI app startup times
added RunUITests task (similar to unInstrumentationTests) to start
app/activity
renamed .UnitTestApk. to .TestApk. as it handles also the new
UI test
updated ProcessLogcatTiming to be able to process logcat output of
started Android activity process

radekdoulik · 2017-09-05T12:00:35Z

the build will fail as it will need a fix from #815 to be able to use custom timing definitions

jonpryor · 2017-09-05T19:17:38Z

PR #515 has been merged.

Indentation is...weirdly all over the map. It should be corrected.

jonpryor · 2017-09-05T19:18:06Z

src/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/RunUITests.cs

+		public                  string              AdbTarget                   { get; set; }
+		public                  string              AdbOptions                  { get; set; }
+
+        [Required]


This should be left-aligned with the public on the following line.

jonpryor · 2017-09-05T19:18:50Z

tests/forms/App.xaml.cs

+
+namespace forms
+{
+    public partial class App : Application


I take it that the default Xamarin.Forms template uses spaces? Not sure if this needs to be fixed, just seems weird.

jonpryor · 2017-09-05T19:19:15Z

tests/forms/Droid/MainActivity.cs

+    public class MainActivity : global::Xamarin.Forms.Platform.Android.FormsAppCompatActivity
+    {
+        bool firstOnCreate = true;
+		bool firstOnStart = true;


The bool on this and the following line should be indented the same amount as the previous line.

jonpryor · 2017-09-05T19:19:39Z

tests/forms/Droid/MainActivity.cs

+                Console.WriteLine("startup-timing: OnCreate end reached");
+                firstOnCreate = false;
+            }
+		}


This line is indented too much; it should be aligned with the { for OnCreate().

jonpryor · 2017-09-05T19:20:22Z

tests/forms/Droid/MainActivity.cs

+                Console.WriteLine("startup-timing: OnStart end reached");
+                firstOnStart = false;
+            }
+		}


This line is indented too much; it should be aligned with the { for OnStart().

jonpryor · 2017-09-05T19:20:37Z

tests/forms/Droid/MainActivity.cs

+        {
+            if (firstOnStart)
+                Console.WriteLine("startup-timing: OnStart reached");
+			base.OnStart();


This line is indented too much; it should be aligned with the if, two lines above.

jonpryor · 2017-09-05T19:21:18Z

tests/forms/Droid/Resources/values/styles.xml

+		<!-- colorPrimaryDark is used for the status bar -->
+		<item name="colorPrimaryDark">#1976D2</item>
+		<!-- colorAccent is used as the default value for colorControlActivated
+         which is used to tint widgets -->


What made this line wrap decision? It's ugly. :-/

jonpryor · 2017-09-05T19:21:42Z

tests/forms/Services/CloudDataStore.cs

+        public async Task<IEnumerable<Item>> GetItemsAsync(bool forceRefresh = false)
+        {
+            if (forceRefresh && CrossConnectivity.Current.IsConnected)
+            {


This { should be on the previous line.

jonpryor · 2017-09-05T19:21:49Z

tests/forms/Services/CloudDataStore.cs

+        public async Task<Item> GetItemAsync(string id)
+        {
+            if (id != null && CrossConnectivity.Current.IsConnected)
+            {


jonpryor · 2017-09-05T19:22:28Z

tests/forms/ViewModels/AboutViewModel.cs

+        {
+            Title = "About";
+
+            OpenWebCommand = new Command(() => Device.OpenUri(new Uri("https://xamarin.com/platform")));


Space before (.

jonpryor · 2017-09-05T19:22:45Z

tests/forms/ViewModels/BaseViewModel.cs

+
+        bool isBusy = false;
+        public bool IsBusy
+        {


This should be on the previous line.

jonpryor · 2017-09-05T19:22:58Z

tests/forms/ViewModels/BaseViewModel.cs

+
+        string title = string.Empty;
+        public string Title
+        {


jonpryor · 2017-09-05T19:23:20Z

tests/forms/ViewModels/ItemsViewModel.cs

+            LoadItemsCommand = new Command(async () => await ExecuteLoadItemsCommand());
+
+            MessagingCenter.Subscribe<NewItemPage, Item>(this, "AddItem", async (obj, item) =>
+            {


This { should be on the previous line.

jonpryor · 2017-09-05T19:23:29Z

tests/forms/ViewModels/ItemsViewModel.cs

+        {
+            Title = "Browse";
+            Items = new ObservableCollection<Item>();
+            LoadItemsCommand = new Command(async () => await ExecuteLoadItemsCommand());


Space before (.

jonpryor · 2017-09-05T19:23:40Z

tests/forms/ViewModels/ItemsViewModel.cs

+            });
+        }
+
+        async Task ExecuteLoadItemsCommand()


Space before (.

jonpryor · 2017-09-05T19:24:17Z

tests/forms/Views/MainPage.cs

+            {
+                case Device.iOS:
+                    itemsPage = new NavigationPage(new ItemsPage())
+                    {


This { should be on the previous line.

jonpryor · 2017-09-05T19:29:40Z

Overall, I don't like the name tests/forms. It doesn't have any semantic meaning, to answer "why does this directory exist?" (Compare to tests/CodeGen-*, which deals with code generation, or tests/ResolveImports, which deals with project resolution.) "forms" is a what, not a why.

I would prefer tests/Xamarin.Forms-Performace-Integration, or something otherwise semantically meaningful.

radekdoulik · 2017-09-06T18:26:56Z

rebased on current master, so hopefully it will build OK now.

regenerated the test with mono coding style set in VisualStudio/Mac, so hopefully it looks better now. also renamed the test directory and package name as suggested.

jonpryor · 2017-09-06T22:08:26Z

build

jonpryor · 2017-09-07T10:37:59Z

One downside to renaming @(UnitTestApk) to @(TestApk) is that this will require additional monodroid-side changes in order to merge. Is this desirable?

jonpryor · 2017-09-07T10:40:56Z

tests/Xamarin.Forms-Performance-Integration/Xamarin.Forms.Performance.Integration.sln

@@ -0,0 +1,19 @@
+


This file shouldn't exist. Instead, the .csproj should be added to the top level Xamarin.Android-Tests.sln file.

Good catch, updated.

radekdoulik · 2017-09-07T11:39:59Z

The monodroid changes are simple, created https://github.com/xamarin/monodroid/pull/665 to do that. It can be completed with adding XA bump.

- to measure UI app startup times - added RunUITests task (similar to unInstrumentationTests) to start app/activity - renamed .*UnitTestApk.* to .*TestApk.* as it handles also the new UI test - updated ProcessLogcatTiming to be able to process logcat output of started Android activity process

radekdoulik requested a review from jonpryor September 5, 2017 11:59

dnfclas added the cla-already-signed label Sep 5, 2017

jonpryor reviewed Sep 5, 2017

View reviewed changes

tests/forms/ViewModels/BaseViewModel.cs Outdated

bool isBusy = false;

public bool IsBusy

{

Copy link

Contributor

jonpryor Sep 5, 2017

Choose a reason for hiding this comment

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

This should be on the previous line.

jonpryor reviewed Sep 5, 2017

View reviewed changes

tests/forms/ViewModels/BaseViewModel.cs Outdated

string title = string.Empty;

public string Title

{

Copy link

Contributor

jonpryor Sep 5, 2017

Choose a reason for hiding this comment

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

Ditto.

jonpryor reviewed Sep 5, 2017

View reviewed changes

tests/forms/ViewModels/ItemsViewModel.cs Outdated

});

}

async Task ExecuteLoadItemsCommand()

Copy link

Contributor

jonpryor Sep 5, 2017

Choose a reason for hiding this comment

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

Space before (.

jonpryor reviewed Sep 5, 2017

View reviewed changes

radekdoulik force-pushed the pr-add-forms-test branch from cfaaf06 to c766a6b Compare September 6, 2017 18:21

radekdoulik force-pushed the pr-add-forms-test branch from c766a6b to e2e6c52 Compare September 6, 2017 18:50

jonpryor reviewed Sep 7, 2017

View reviewed changes

radekdoulik force-pushed the pr-add-forms-test branch from e2e6c52 to d0a5c2d Compare September 7, 2017 11:34

radekdoulik force-pushed the pr-add-forms-test branch from d0a5c2d to 86d6af0 Compare September 7, 2017 12:07

radekdoulik force-pushed the pr-add-forms-test branch from 86d6af0 to 2e8966e Compare September 7, 2017 12:56

jonpryor merged commit 09ba1ee into dotnet:master Sep 7, 2017

github-actions bot locked and limited conversation to collaborators Feb 2, 2024

[tests] added forms test #818

[tests] added forms test #818

Uh oh!

Conversation

radekdoulik commented Sep 5, 2017

Uh oh!

radekdoulik commented Sep 5, 2017

Uh oh!

jonpryor commented Sep 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonpryor commented Sep 5, 2017

Uh oh!

radekdoulik commented Sep 6, 2017

Uh oh!

jonpryor commented Sep 6, 2017

Uh oh!

jonpryor commented Sep 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radekdoulik commented Sep 7, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants