[GitHub] [tvm] areusch commented on a change in pull request #10302: [CI][Hexagon] Add Hexagon Tests to pipeline

[GitBox <gi...@apache.org>]

areusch commented on a change in pull request #10302:
URL: https://github.com/apache/tvm/pull/10302#discussion_r812364021



##########
File path: tests/python/contrib/test_hexagon/conftest.py
##########
@@ -49,83 +49,26 @@ def _compose(args, decs):
 def requires_hexagon_toolchain(*args):
     _requires_hexagon_toolchain = [
         pytest.mark.skipif(
-            os.environ.get("HEXAGON_TOOLCHAIN") == None,
-            reason="HEXAGON_TOOLCHAIN environment variable is required to run this test.",
+            os.environ.get(HEXAGON_TOOLCHAIN) == None,
+            reason=f"Missing environment variable, {HEXAGON_TOOLCHAIN}.",

Review comment:
       nit: remove the `,` or replace with `:`

##########
File path: tests/python/contrib/test_hexagon/test_launcher.py
##########
@@ -50,6 +49,10 @@ def test_add(tvm_tracker_host, tvm_tracker_port, android_serial_number):
     dso_binary_path = temp.relpath(dso_binary)
     func.save(dso_binary_path)
 
+    if not android_serial_number:
+        logging.info("Skip hardware test since ANDROID_SERIAL_NUMBER is not set.")

Review comment:
       can this be `pytest.skip()`? here and below

##########
File path: tests/scripts/task_python_hexagon.sh
##########
@@ -15,4 +16,12 @@
 # specific language governing permissions and limitations
 # under the License.
 
-""" Testing infrastructure for Hexagon RPC"""
+set -e
+set -u
+
+source tests/scripts/setup-pytest-env.sh
+
+make cython3
+
+unset ANDROID_SERIAL_NUMBER

Review comment:
       why do we need this unset here? can you add a comment?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org