[PATCH] D78955: [zorg] Add polly test-suite builder.

[Galina via Phabricator via llvm-commits]

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

Please see inline comments.



================
Comment at: zorg/buildbot/builders/PollyBuilder.py:17
     extraCmakeArgs=None,
+    testsuite=False,extraTestsuiteCmakeArgs=None,
     **kwargs):
----------------
Is the new line is missing between the arguments?


================
Comment at: zorg/buildbot/builders/PollyBuilder.py:65
+    if testsuite:
+        # Get the builddir for use with WithProperties
+        f.addStep(SetProperty(name="get_builddir",
----------------
You do not need a step to get the builddir. This is `workdir` property, just use that. For example,
```
WithProperties("%(workdir)s/" + llvm_instdir + "/bin/clang++")
```
would give you a fully qualified path to the installed clang++.


================
Comment at: zorg/buildbot/builders/PollyBuilder.py:114
 
+    clangexe   = os.path.join(llvm_objdir, 'bin', 'clang')
+    clangxxexe = os.path.join(llvm_objdir, 'bin', 'clang++')
----------------
This code will be running on the master. The path on master has nothing to do with the path on the bot which will be actually building. Here and in thew rest of similar places, please use forward slashes here as it would work everywhere.


================
Comment at: zorg/buildbot/builders/PollyBuilder.py:181
+                                    WithProperties("-DTEST_SUITE_LIT=%(builddir)s/" + litexe),
+                                    WithProperties("-DTEST_SUITE_LIT_FLAGS=-v;-o;report.json"),
+                                    WithProperties("-DTEST_SUITE_LLVM_SIZE=%(builddir)s/" + sizeexe)
----------------
Adding -vv lit flag might help troubleshooting test failures.


Repository:
  rZORG LLVM Github Zorg

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78955/new/

https://reviews.llvm.org/D78955