[PATCH] D27766: [buildbot] Added CUDATestsuiteBuilder.py

Galina via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 17 23:09:33 PST 2016


gkistanova requested changes to this revision.
gkistanova added a comment.
This revision now requires changes to proceed.

Thanks for working on this, Artem.



================
Comment at: zorg/buildbot/builders/CUDATestsuiteBuilder.py:16
+        cmake='cmake',
+        extra_cmake_args=[],  # Extra CMake args for all stages.
+        extra_ts_cmake_args=[],  # extra cmake args for testsuite.
----------------
Please do not use mutable default arguments. 


================
Comment at: zorg/buildbot/builders/CUDATestsuiteBuilder.py:52
+    f = ClangBuilder.getClangCMakeBuildFactory(
+        clean=always_clean,
+        test=test,
----------------
Maybe adding an extra indent here? This is cosmetic, though.


================
Comment at: zorg/buildbot/builders/CUDATestsuiteBuilder.py:101
+    f.addStep(ShellCommand(name='cmake test-suite',
+                           command=cmake_ts_cmd,
+                           haltOnFailure=True,
----------------
Could you use CmakeCommand instead of ShellCommand here, please?


================
Comment at: zorg/buildbot/builders/CUDATestsuiteBuilder.py:113
+    f.addStep(WarningCountingShellCommand(
+        name='ninja build simple CUDA tests',
+        command=ninja_build_cmd + ["cuda-tests-simple"],
----------------
Could you use NinjaCommand instead of WarningCountingShellCommand here and in all the other similar places, please?


https://reviews.llvm.org/D27766





More information about the llvm-commits mailing list