Minor tweaks to code and tests

.yamato/upm-ci.yml

@@ -44,7 +44,7 @@ test_{{ platform.name }}_{{ editor.version }}_{{ module_support.name }}:
     {% if platform.model %}model: {{ platform.model }}{% endif %}
   commands:
     - unity-downloader-cli -u {{ editor.version }} {{ platform.install_editor_command }} {{ module_support.install_command }} --wait
-    - {{ platform.move_alias }} ./disable_tests_csc.rsp ./com.unity.mobile.android-logcat/Tests/Editor/Integration/csc.rsp
+    - python .yamato/use-packed-package.py TestProjects/SampleProject1 com.unity.mobile.android-logcat
     - {{ platform.utr_cmd }}  --suite=editor --editor-location=.Editor --testproject="TestProjects/SampleProject1" --extra-editor-arg=-buildTarget --extra-editor-arg=Android --artifacts_path=upm-ci~/test-results/editor-android/
     - {{ platform.utr_cmd }}  --suite=editor --editor-location=.Editor --testproject="TestProjects/TestWarnings" --artifacts_path=upm-ci~/test-results/editor-warnings-android/
   artifacts:

.yamato/upm-integration-ci.yml

@@ -3,26 +3,27 @@
 ---
 {% for editor in test_editors %}
 test_integration_{{ editor.version }}:
-  name : Test Integration on {{ editor.version }}
+  name : Test Integration on {{ editor.version }} Pixel Pro Android 16
   agent:
-      type: Unity::mobile::shield
-      image: mobile/android-package-ci-win:latest
-      flavor: b1.medium
+      image: mobile/android-windows-unity:v2.8585035
+      type: Unity::mobile::pixel
+      flavor: b1.large
+      model: 8pro-Android16
   commands:
-    - pip install unity-downloader-cli --index-url https://artifactory.prd.it.unity3d.com/artifactory/api/pypi/pypi/simple --upgrade
+    - gsudo choco install unity-downloader-cli -y -s https://artifactory.prd.it.unity3d.com/artifactory/api/nuget/unity-choco-local
     - unity-downloader-cli -u {{ editor.version }} -c editor -c android --wait
     - curl -s https://artifactory.prd.it.unity3d.com/artifactory/unity-tools-local/utr-standalone/utr.bat --output utr.bat
-    - move ./disable_tests_csc.rsp ./com.unity.mobile.android-logcat/Tests/Editor/csc.rsp
+    - python .yamato/use-packed-package.py TestProjects/SampleProject1 com.unity.mobile.android-logcat
     - |
-       REM Set the IP of the device. In case device gets lost, UTR will try to recconect to ANDROID_DEVICE_CONNECTION
-       set ANDROID_DEVICE_CONNECTION=%BOKKEN_DEVICE_IP%
        set ARTIFACTS_PATH=upm-ci~/test-results/integration-artifacts/
        REM Editor will perform the connection instead of upm script, since editor might use different SDK (and adb)
        ./utr.bat  --suite=editor --editor-location=.Editor --testproject="TestProjects/SampleProject1" --artifacts_path=upm-ci~/test-results/editor-android/
   artifacts:
     integration_{{ editor.version }}_logs:
       paths:
         - "upm-ci~/test-results/**/*"
+  variables:
+    ANDROID_DEVICE_AVAILABLE: 1
   dependencies:
     - .yamato/wrench/package-pack-jobs.yml#package_pack_-_mobile_android-logcat
 {% endfor %}

.yamato/use-packed-package.py

@@ -0,0 +1,35 @@
+"""Points a test project at the packed package instead of the source folder.
+
+The source folder has no server jar: it is a build output, gitignored, and built by
+the jar job whose artifact the pack job folds into the tarball. Tests that push the
+jar to a device therefore need the package the pack job produced - which is also what
+a user installs, so this is the thing worth testing.
+
+Usage: python .yamato/use-packed-package.py <project path> <package name>
+"""
+import glob
+import json
+import os
+import sys
+
+project, package = sys.argv[1], sys.argv[2]
+
+# Globbed rather than named with a version, the way the wrench validation jobs take
+# their packages, so a version bump does not have to reach in here.
+tarballs = glob.glob(os.path.join('upm-ci~', 'packages', package + '-*.tgz'))
+if len(tarballs) != 1:
+    found = ', '.join(sorted(tarballs)) or 'nothing'
+    sys.exit(f"Expected one {package} tarball from the pack job, found {found}.")
+
+manifest_path = os.path.join(project, 'Packages', 'manifest.json')
+with open(manifest_path, encoding='utf-8') as f:
+    manifest = json.load(f)
+
+previous = manifest['dependencies'].get(package)
+manifest['dependencies'][package] = 'file:' + os.path.abspath(tarballs[0]).replace('\\', '/')
+
+with open(manifest_path, 'w', encoding='utf-8') as f:
+    json.dump(manifest, f, indent=2)
+    f.write('\n')
+
+print(f"{manifest_path}: {package} {previous} -> {manifest['dependencies'][package]}")

com.unity.mobile.android-logcat/Editor/AndroidLogcatCaptureVideo.cs

@@ -45,7 +45,7 @@ private void Cleanup()
             var device = m_RecordingOnDevice;
             StopRecording();
             DeleteVideoOnDevice(device);
-            KillRemoteRecorder(m_Runtime, device);
+            AndroidLogcatUtilities.KillScreenRecordProcess(m_Runtime, device);
             m_Runtime = null;
         }
 
@@ -94,15 +94,6 @@ internal bool IsRemoteRecorderActive(IAndroidLogcatDevice device)
             return AndroidLogcatUtilities.GetPidFromPackageName(m_Runtime.Tools.ADB, device, "screenrecord") != -1;
         }
 
-        internal static void KillRemoteRecorder(AndroidLogcatRuntimeBase runtime, IAndroidLogcatDevice device)
-        {
-            if (device == null)
-                return;
-            var pid = AndroidLogcatUtilities.GetPidFromPackageName(runtime.Tools.ADB, device, "screenrecord");
-            if (pid != -1)
-                AndroidLogcatUtilities.KillProcesss(runtime.Tools.ADB, device, pid);
-        }
-
         private void DeleteVideoOnHost(string path)
         {
             try
@@ -155,7 +146,7 @@ internal void StartRecording(IAndroidLogcatDevice device,
             m_RecordingOnDevice = device;
 
             DeleteVideoOnHost(GetVideoPath(device));
-            KillRemoteRecorder(m_Runtime, m_RecordingOnDevice);
+            AndroidLogcatUtilities.KillScreenRecordProcess(m_Runtime, m_RecordingOnDevice);
 
             // If for some reason screen recorder is still running, abort.
             if (IsRemoteRecorderActive(m_RecordingOnDevice))
@@ -249,7 +240,7 @@ private bool CollectRecording(string targetPath)
             if (!CopyVideoFromDevice(m_RecordingOnDevice, targetPath))
             {
                 result = false;
-                KillRemoteRecorder(m_Runtime, m_RecordingOnDevice);
+                AndroidLogcatUtilities.KillScreenRecordProcess(m_Runtime, m_RecordingOnDevice);
             }
 
             DeleteVideoOnDevice(m_RecordingOnDevice);

com.unity.mobile.android-logcat/Editor/AndroidLogcatDevice.cs

@@ -7,6 +7,19 @@
 
 namespace Unity.Android.Logcat
 {
+    /// <summary>
+    /// How the device's screen is rotated. The numbers are Android's own
+    /// Surface.ROTATION_* values, which is what the user_rotation setting takes.
+    /// </summary>
+    internal enum AndroidDeviceRotation
+    {
+        Auto = -1,
+        Rotate0 = 0,
+        Rotate90 = 1,
+        Rotate180 = 2,
+        Rotate270 = 3
+    }
+
     internal abstract class IAndroidLogcatDevice
     {
         internal IAndroidLogcatActivityManager m_ActivityManager;
@@ -53,6 +66,30 @@ internal enum DeviceState
 
         internal abstract void QueryDisplaySize(out Vector2 displaySize, out Vector2? overridenDisplaySize);
 
+        /// <summary>
+        /// Wakes the device's screen. A display that is off composes nothing, so
+        /// anything that reads the screen - a mirrored display, screenrecord - gets
+        /// nothing at all out of a sleeping device.
+        /// <para>
+        /// Only a wake: a lock screen stays up, and streams perfectly well, because it
+        /// composes like any other screen. Best effort, too - a device that will not
+        /// take it is not an error, since it may well be showing something already.
+        /// </para>
+        /// </summary>
+        internal abstract void WakeUp();
+
+        /// <summary>
+        /// Puts the device's screen to sleep, the counterpart of <see cref="WakeUp"/>
+        /// and best effort in the same way.
+        /// </summary>
+        internal abstract void Sleep();
+
+        /// <summary>
+        /// Rotates the device's screen, or with <see cref="AndroidDeviceRotation.Auto"/>
+        /// hands the rotation back to the accelerometer.
+        /// </summary>
+        internal abstract void SetRotation(AndroidDeviceRotation rotation);
+
         protected void ParseDisplaySize(string input, out Vector2 displaySize, out Vector2? overridenDisplaySize)
         {
             displaySize = Vector2.zero;
@@ -154,6 +191,7 @@ internal class AndroidLogcatDevice : IAndroidLogcatDevice
         private AndroidBridge.ADB m_ADB;
         private Version m_Version;
         private string m_DisplayName;
+
         internal AndroidLogcatDevice(AndroidBridge.ADB adb, string deviceId)
             : base(new AndroidLogcatActivityManager(adb, deviceId))
         {
@@ -257,6 +295,48 @@ internal override string DisplayName
             }
         }
 
+        internal override void WakeUp() => SendPowerKey("KEYCODE_WAKEUP", "Failed to wake the device");
+
+        internal override void Sleep() => SendPowerKey("KEYCODE_SLEEP", "Failed to put the device to sleep");
+
+        void SendPowerKey(string keyCode, string failureMessage)
+        {
+            if (m_Device == null || State != DeviceState.Connected)
+                return;
+
+            var args = $"-s {Id} shell input keyevent {keyCode}";
+            try
+            {
+                var output = m_ADB.Run(new[] { args }, failureMessage);
+                AndroidLogcatInternalLog.Log($"adb {args}\n{output}");
+            }
+            catch (Exception ex)
+            {
+                AndroidLogcatInternalLog.Log(ex.Message);
+            }
+        }
+
+        internal override void SetRotation(AndroidDeviceRotation rotation)
+        {
+            if (m_Device == null || State != DeviceState.Connected)
+                return;
+
+            // user_rotation is only obeyed while auto rotation is off, so the two
+            // settings are one operation: turn the accelerometer off and pin the
+            // rotation, or turn it back on and leave the pinned value alone.
+            var auto = rotation == AndroidDeviceRotation.Auto;
+            PutSystemSetting("accelerometer_rotation", auto ? 1 : 0);
+            if (!auto)
+                PutSystemSetting("user_rotation", (int)rotation);
+        }
+
+        void PutSystemSetting(string name, int value)
+        {
+            var args = $"-s {Id} shell settings put system {name} {value}";
+            AndroidLogcatInternalLog.Log($"adb {args}");
+            m_ADB.Run(new[] { args }, $"Failed to set '{name}' to {value}");
+        }
+
         internal override void QueryDisplaySize(out Vector2 displaySize, out Vector2? overridenDisplaySize)
         {
             overridenDisplaySize = null;

com.unity.mobile.android-logcat/Editor/AndroidLogcatUtilities.cs

@@ -72,16 +72,23 @@ public static bool CaptureScreen(AndroidBridge.ADB adb, string deviceId, string
 
         public static string GetTemporaryPath(IAndroidLogcatDevice device, string name, string extension)
         {
-            string fileName = device != null ? device.Id : "NoDevice";
-            if (device != null)
-            {
-                foreach (var p in Path.GetInvalidFileNameChars())
-                    fileName = fileName.Replace(p, '_');
-            }
+            string fileName = device != null ? SanitizeFileName(device.Id) : "NoDevice";
             fileName = $"{name}_{fileName}{extension}";
             return Path.Combine(Application.dataPath, "..", "Temp", fileName).Replace("\\", "/");
         }
 
+        /// <summary>
+        /// Replaces anything the filesystem will not accept in a file name. A device id
+        /// can be an ip:port, and ':' is not allowed on Windows.
+        /// </summary>
+        public static string SanitizeFileName(string name)
+        {
+            foreach (var c in Path.GetInvalidFileNameChars())
+                name = name.Replace(c, '_');
+            return name;
+        }
+
+
         /// <summary>
         /// Get the top activity on the given device.
         /// </summary>
@@ -483,6 +490,15 @@ internal static bool ParseCrashLine(IReadOnlyList<ReordableListItem> regexs, str
             return false;
         }
 
+        internal static void KillScreenRecordProcess(AndroidLogcatRuntimeBase runtime, IAndroidLogcatDevice device)
+        {
+            if (device == null)
+                return;
+            var pid = GetPidFromPackageName(runtime.Tools.ADB, device, "screenrecord");
+            if (pid != -1)
+                KillProcesss(runtime.Tools.ADB, device, pid);
+        }
+
         internal static void ShowAndroidIsNotInstalledMessage()
         {
             UnityEditor.EditorGUILayout.HelpBox("Android Logcat requires Android support to be installed.", UnityEditor.MessageType.Info);

com.unity.mobile.android-logcat/Editor/AndroidTools/AndroidBridge.cs

@@ -212,6 +212,7 @@ internal class AndroidExternalToolsSettings
             private static Type s_AndroidExternalToolsSettingsType;
             private static PropertyInfo s_NdkRootPathProperty;
             private static PropertyInfo s_SdkRootPathProperty;
+            private static PropertyInfo s_JdkRootPathProperty;
 
             private static Type UnderlyingType
             {
@@ -251,6 +252,17 @@ private static PropertyInfo SdkRootPathProperty
                 }
             }
 
+            private static PropertyInfo JdkRootPathProperty
+            {
+                get
+                {
+                    if (s_JdkRootPathProperty != null)
+                        return s_JdkRootPathProperty;
+                    s_JdkRootPathProperty = UnderlyingType.GetProperty("jdkRootPath");
+                    return s_JdkRootPathProperty;
+                }
+            }
+
             /// <summary>
             /// Matches to UnityEditor.Android.AndroidExternalToolsSettings.ndkRootPath
             /// </summary>
@@ -268,6 +280,15 @@ public static string sdkRootPath
                 get => (string)SdkRootPathProperty.GetValue(null);
                 set => SdkRootPathProperty.SetValue(null, value);
             }
+
+            /// <summary>
+            /// Matches to UnityEditor.Android.AndroidExternalToolsSettings.jdkRootPath
+            /// </summary>
+            public static string jdkRootPath
+            {
+                get => (string)JdkRootPathProperty.GetValue(null);
+                set => JdkRootPathProperty.SetValue(null, value);
+            }
         }
     }
 }

com.unity.mobile.android-logcat/Tests/Editor/AndroidLogcatFakeDevice.cs

@@ -29,6 +29,21 @@ internal override string Id
         get { return m_DeviceId; }
     }
 
+    /// <summary>Nothing to wake.</summary>
+    internal override void WakeUp()
+    {
+    }
+
+    /// <summary>Nothing to put to sleep.</summary>
+    internal override void Sleep()
+    {
+    }
+
+    /// <summary>Nothing to rotate.</summary>
+    internal override void SetRotation(AndroidDeviceRotation rotation)
+    {
+    }
+
     internal override void QueryDisplaySize(out Vector2 displaySize, out Vector2? overridenDisplaySize)
     {
         ParseDisplaySize(m_DisplayInfo, out displaySize, out overridenDisplaySize);
@@ -39,9 +54,9 @@ internal void SetRawDisplayInfo(string displayInfo)
         m_DisplayInfo = displayInfo;
     }
 
-    internal override string DisplayName => throw new NotImplementedException();
+    internal override string DisplayName => throw new NotImplementedException(nameof(DisplayName));
 
-    internal override string ShortDisplayName => throw new NotImplementedException();
+    internal override string ShortDisplayName => throw new NotImplementedException(nameof(ShortDisplayName));
 
     protected override string GetTagPriorityAsString(string tag)
     {