From 9212b7f0515028fae95c36c611ca7c4364b5f9a8 Mon Sep 17 00:00:00 2001
From: Luis Machado <luis.machado@arm.com>
Date: Fri, 26 Jul 2024 15:44:22 +0100
Subject: [PATCH] [workload/JetNews] Address reviewer comments

- Add JetNews workload description.
- Fixup copyright years.
- Adjust formatting.
- Add constraints to the workload's parameters.
- Fix comment typos.
- Remove unused commented-out code from app/build.gradle.
---
 wa/workloads/jetnews/__init__.py              | 30 ++++++++++++-------
 wa/workloads/jetnews/uiauto/app/build.gradle  |  1 -
 .../arm/wa/uiauto/jetnews/UiAutomation.java   |  4 +--
 .../uiauto/jetnews/UiAutomationJankTests.java |  2 +-
 4 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/wa/workloads/jetnews/__init__.py b/wa/workloads/jetnews/__init__.py
index d9f2d671..7bc2fa93 100755
--- a/wa/workloads/jetnews/__init__.py
+++ b/wa/workloads/jetnews/__init__.py
@@ -14,7 +14,9 @@
 #
 
 from wa import ApkUiautoWorkload, Parameter, TestPackageHandler
+
 from wa.utils.types import list_of_strs
+
 import re
 
 class Jetnews(ApkUiautoWorkload):
@@ -22,9 +24,22 @@ class Jetnews(ApkUiautoWorkload):
     name = 'jetnews'
     package_names = ['com.example.jetnews']
     description = '''
-    JetNews
+    This workload uses the JetNews sample app to run a set of UiAutomation
+    tests, with the goal of gathering frame metrics and calculating jank
+    frame percentages.
+
+    It uses two APK's, the JetNews app itself (modified to contain more posts)
+    and the UiAutomation tests that interact with the app.
+
+    There are 3 available tests, two in portrait mode and 1 in landscape mode.
     '''
 
+    _OUTPUT_SECTION_REGEX = re.compile(
+        r'(\s*INSTRUMENTATION_STATUS: gfx-[\w-]+=[-+\d.]+\n)+'
+        r'\s*INSTRUMENTATION_STATUS_CODE: (?P<code>[-+\d]+)\n?', re.M)
+    _OUTPUT_GFXINFO_REGEX = re.compile(
+        r'INSTRUMENTATION_STATUS: (?P<name>[\w-]+)=(?P<value>[-+\d.]+)')
+
     default_test_strings = [
         'PortraitVerticalTest',
         'PortraitHorizontalTest',
@@ -38,25 +53,20 @@ class Jetnews(ApkUiautoWorkload):
                   tests are PortraitVerticalTest, LandscapeVerticalTest and
                   PortraitHorizontalTest. If none are specified, the default
                   is to run all of them.
-                  """, default=default_test_strings),
+                  """, default=default_test_strings,
+                  constraint=lambda x: all(v in ['PortraitVerticalTest', 'PortraitHorizontalTest', 'LandscapeVerticalTest'] for v in x)),
         Parameter('flingspeed', kind=int,
                   description="""
                   Default fling speed for the tests. The default is 5000 and
                   the minimum value is 1000.
-                  """, default=5000),
+                  """, default=5000, constraint=lambda x: x >= 1000),
         Parameter('repeat', kind=int,
                   description="""
                   The number of times the tests should be repeated. The default
                   is 1.
-                  """, default=1)
+                  """, default=1, constraint=lambda x: x > 0)
     ]
 
-    _OUTPUT_SECTION_REGEX = re.compile(
-        r'(\s*INSTRUMENTATION_STATUS: gfx-[\w-]+=[-+\d.]+\n)+'
-        r'\s*INSTRUMENTATION_STATUS_CODE: (?P<code>[-+\d]+)\n?', re.M)
-    _OUTPUT_GFXINFO_REGEX = re.compile(
-        r'INSTRUMENTATION_STATUS: (?P<name>[\w-]+)=(?P<value>[-+\d.]+)')
-
     def __init__(self, target, **kwargs):
         super(Jetnews, self).__init__(target, **kwargs)
         # This test uses the androidx library.
diff --git a/wa/workloads/jetnews/uiauto/app/build.gradle b/wa/workloads/jetnews/uiauto/app/build.gradle
index 399b2cbd..d612bc82 100644
--- a/wa/workloads/jetnews/uiauto/app/build.gradle
+++ b/wa/workloads/jetnews/uiauto/app/build.gradle
@@ -8,7 +8,6 @@ kotlin {
     jvmToolchain(17)
 }
 
-//apply plugin: 'com.android.application'
 def packageName = "com.arm.wa.uiauto.jetnews"
 
 android {
diff --git a/wa/workloads/jetnews/uiauto/app/src/main/java/com/arm/wa/uiauto/jetnews/UiAutomation.java b/wa/workloads/jetnews/uiauto/app/src/main/java/com/arm/wa/uiauto/jetnews/UiAutomation.java
index 275dd8ce..ae0f28e3 100644
--- a/wa/workloads/jetnews/uiauto/app/src/main/java/com/arm/wa/uiauto/jetnews/UiAutomation.java
+++ b/wa/workloads/jetnews/uiauto/app/src/main/java/com/arm/wa/uiauto/jetnews/UiAutomation.java
@@ -1,4 +1,4 @@
-/*    Copyright 2014-2024 ARM Limited
+/*    Copyright 2024 ARM Limited
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,7 +26,7 @@ import com.arm.wa.uiauto.UiAutoUtils;
 
 import org.junit.Test;
 
-// Dummy workload for jetnews. We need to use JankTestBasem but we
+// Dummy workload for jetnews. We need to use the JankTestBase class but we
 // can't inherit from that class as we already inherit BaseUiAutomation.
 // Therefore we have another class (UiAutomationJankTests) that uses
 // this class instead.
diff --git a/wa/workloads/jetnews/uiauto/app/src/main/java/com/arm/wa/uiauto/jetnews/UiAutomationJankTests.java b/wa/workloads/jetnews/uiauto/app/src/main/java/com/arm/wa/uiauto/jetnews/UiAutomationJankTests.java
index ce5be318..e8da7140 100644
--- a/wa/workloads/jetnews/uiauto/app/src/main/java/com/arm/wa/uiauto/jetnews/UiAutomationJankTests.java
+++ b/wa/workloads/jetnews/uiauto/app/src/main/java/com/arm/wa/uiauto/jetnews/UiAutomationJankTests.java
@@ -1,4 +1,4 @@
-/*    Copyright 2014-2024 ARM Limited
+/*    Copyright 2024 ARM Limited
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.