Skip to content

Commit b019cbc

Browse files
committed
[java-driver#755] Fixing issue when skipped exception is treated as failed one.
1 parent 0f3ae37 commit b019cbc

File tree

2 files changed

+46
-37
lines changed

2 files changed

+46
-37
lines changed

driver-core/src/test/java/com/datastax/driver/core/CCMTestsSupport.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,11 @@ public void beforeTestClass() throws Exception {
637637
* @throws Exception
638638
*/
639639
public void beforeTestClass(Object testInstance) throws Exception {
640+
// Check for class-level skip conditions before setting up CCM cluster
641+
// Workaround for testng 6.13.x bug ensuring that test clusters are not created for skipped
642+
// tests
643+
TestListener.checkForSkipConditions(testInstance.getClass());
644+
640645
testMode = determineTestMode(testInstance.getClass());
641646
if (testMode == PER_CLASS) {
642647
closer = Closer.create();

driver-core/src/test/java/com/datastax/driver/core/TestListener.java

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,6 @@ public class TestListener extends TestListenerAdapter implements IInvokedMethodL
4545

4646
@Override
4747
public void onTestFailure(ITestResult tr) {
48-
if (tr.getThrowable() instanceof SkipException) {
49-
// Workaround for testng 6.13.x bug https://github.com/testng-team/testng/issues/1632
50-
// When SkipException thrown from beforeInvocation marks test as FAILED
51-
tr.setStatus(ITestResult.SKIP);
52-
return;
53-
}
5448
long elapsedTime = TimeUnit.NANOSECONDS.toSeconds((System.nanoTime() - start_time));
5549
long testTime = tr.getEndMillis() - tr.getStartMillis();
5650
tr.getThrowable().printStackTrace();
@@ -120,19 +114,34 @@ public void beforeInvocation(IInvokedMethod testMethod, ITestResult testResult)
120114
ITestNGMethod testNgMethod = testResult.getMethod();
121115
ConstructorOrMethod constructorOrMethod = testNgMethod.getConstructorOrMethod();
122116

123-
Class<?> clazz = testNgMethod.getInstance().getClass();
124-
if (clazz != null) {
117+
try {
118+
Class<?> clazz = testNgMethod.getInstance().getClass();
125119
do {
126-
if (scanAnnotatedElement(clazz)) break;
120+
// Check for skip conditions and break early if version annotations are found
121+
if (checkForSkipConditions(clazz)) break;
127122
} while (!(clazz = clazz.getSuperclass()).equals(Object.class));
128-
}
129-
Method method = constructorOrMethod.getMethod();
130-
if (method != null) {
131-
scanAnnotatedElement(method);
123+
Method method = constructorOrMethod.getMethod();
124+
if (method != null) {
125+
checkForSkipConditions(method); // Don't need return value for methods
126+
}
127+
} catch (SkipException e) {
128+
// Workaround for testng 6.13.x bug https://github.com/testng-team/testng/issues/1632
129+
// When SkipException thrown from beforeInvocation marks test as FAILED
130+
// Instead of letting TestNG handle it, we manually set the skip status
131+
testResult.setStatus(ITestResult.SKIP);
132+
testResult.setThrowable(e);
133+
testResult.setEndMillis(System.currentTimeMillis());
134+
// Don't re-throw the exception to avoid the bug
132135
}
133136
}
134137

135-
private boolean scanAnnotatedElement(AnnotatedElement element) {
138+
/**
139+
* Static method to check for skip conditions on a class or method. Throws SkipException if the
140+
* element should be skipped.
141+
*
142+
* @return true if version-related annotations were found (to break early in class hierarchy scan)
143+
*/
144+
public static boolean checkForSkipConditions(AnnotatedElement element) {
136145
if (CCMBridge.getGlobalScyllaVersion() != null) {
137146
if (element.isAnnotationPresent(ScyllaSkip.class)) {
138147
throw new SkipException("Skipping test because it is disabled for Scylla cluster.");
@@ -158,8 +167,6 @@ private boolean scanAnnotatedElement(AnnotatedElement element) {
158167
throw new SkipException(
159168
"Skipping test because it is designed for DSE only, but running on Scylla cluster.");
160169
}
161-
162-
return false;
163170
} else if (CCMBridge.isDse()) {
164171
if (element.isAnnotationPresent(ScyllaOnly.class)) {
165172
throw new SkipException("Skipping test because it is enabled only for Scylla cluster.");
@@ -181,31 +188,28 @@ private boolean scanAnnotatedElement(AnnotatedElement element) {
181188
throw new SkipException(
182189
"Skipping test because it is designed for Scylla only, but running on DSE cluster.");
183190
}
191+
} else {
192+
if (element.isAnnotationPresent(ScyllaOnly.class)) {
193+
throw new SkipException("Skipping test because it is enabled only for Scylla cluster.");
194+
}
184195

185-
return false;
186-
}
187-
188-
if (element.isAnnotationPresent(ScyllaOnly.class)) {
189-
throw new SkipException("Skipping test because it is enabled only for Scylla cluster.");
190-
}
191-
192-
if (element.isAnnotationPresent(CassandraVersion.class)) {
193-
CassandraVersion cassandraVersion = element.getAnnotation(CassandraVersion.class);
194-
cassandraVersionCheck(cassandraVersion);
195-
return true;
196-
}
196+
if (element.isAnnotationPresent(CassandraVersion.class)) {
197+
CassandraVersion cassandraVersion = element.getAnnotation(CassandraVersion.class);
198+
cassandraVersionCheck(cassandraVersion);
199+
return true;
200+
}
197201

198-
if (element.isAnnotationPresent(ScyllaVersion.class)) {
199-
throw new SkipException(
200-
"Skipping test because it is designed for Scylla only, but running on Cassandra cluster.");
201-
}
202+
if (element.isAnnotationPresent(ScyllaVersion.class)) {
203+
throw new SkipException(
204+
"Skipping test because it is designed for Scylla only, but running on Cassandra cluster.");
205+
}
202206

203-
if (element.isAnnotationPresent(DseVersion.class)) {
204-
throw new SkipException(
205-
"Skipping test because it is designed for DSE only, but running on Cassandra cluster.");
207+
if (element.isAnnotationPresent(DseVersion.class)) {
208+
throw new SkipException(
209+
"Skipping test because it is designed for DSE only, but running on Cassandra cluster.");
210+
}
206211
}
207-
208-
return false;
212+
return false; // No version annotations found, continue scanning
209213
}
210214

211215
@Override

0 commit comments

Comments
 (0)