ctubbsii commented on code in PR #2038:
URL: https://github.com/apache/zookeeper/pull/2038#discussion_r1303870923
##########
Jenkinsfile-s390x:
##########
@@ -48,7 +48,7 @@ pipeline {
stage('BuildAndTest') {
steps {
sh "git clean -fxd"
- sh "mvn verify spotbugs:check checkstyle:check
-Pfull-build -Dsurefire-forkcount=4"
+ sh "mvn verify spotbugs:check checkstyle:check
-Pfull-build -Dsurefire-forkcount=4 -Dtest=\!ClientSSLTest
-DfailIfNoTests=false"
Review Comment:
Thinking about this a bit more, I think you actually shouldn't skip the test
here. Instead, do it in JUnit. In the ClientSSLTest, you can add:
```java
Assumptions.assumeFalse(System.getProperty("os.arch").contains("s390x"),
"Test cannot run for s390x because ...").
```
If you add something like that to the top of a test case, JUnit will not
only happily skip the test, but it will also helpfully print out an explanation
of why the test was skipped. Keeping the exclusion close to the test case
itself is better than writing the exclusion into the Jenkinsfile.
Also, it's not even clear that this is a good pattern to use... as it will
try to execute any utility classes as test classes, in addition to the default
pattern of `*Test,Test*`.
I think this should be changed to use the JUnit Assumption instead.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
