This is an automated email from the ASF dual-hosted git repository. chesnay pushed a commit to branch release-1.13 in repository https://gitbox.apache.org/repos/asf/flink.git
commit 4fefb626570359fedaaeac10ef855c7caaa307a6 Author: Chesnay Schepler <[email protected]> AuthorDate: Wed Apr 21 14:57:19 2021 +0200 [FLINK-20723][tests] Ignore expected exception when retrying --- .../apache/flink/testutils/junit/RetryRule.java | 52 +++++++++---- .../flink/testutils/junit/RetryRuleTest.java | 88 ++++++++++++++++++++++ 2 files changed, 126 insertions(+), 14 deletions(-) diff --git a/flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/testutils/junit/RetryRule.java b/flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/testutils/junit/RetryRule.java index beb2f24..04fca6a 100644 --- a/flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/testutils/junit/RetryRule.java +++ b/flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/testutils/junit/RetryRule.java @@ -54,32 +54,38 @@ public class RetryRule implements TestRule { RetryOnFailure retryOnFailure = description.getAnnotation(RetryOnFailure.class); RetryOnException retryOnException = description.getAnnotation(RetryOnException.class); - // sanity check that we don't use expected exceptions with the RetryOnX annotations - if (retryOnFailure != null || retryOnException != null) { - Test test = description.getAnnotation(Test.class); - if (test.expected() != Test.None.class) { - throw new IllegalArgumentException( - "You cannot combine the RetryOnFailure " - + "annotation with the Test(expected) annotation."); - } - } - // sanity check that we don't use both annotations if (retryOnFailure != null && retryOnException != null) { throw new IllegalArgumentException( "You cannot combine the RetryOnFailure and RetryOnException annotations."); } + final Class<? extends Throwable> whitelistedException = getExpectedException(description); + if (retryOnFailure != null) { - return new RetryOnFailureStatement(retryOnFailure.times(), statement); + return new RetryOnFailureStatement( + retryOnFailure.times(), statement, whitelistedException); } else if (retryOnException != null) { return new RetryOnExceptionStatement( - retryOnException.times(), retryOnException.exception(), statement); + retryOnException.times(), + retryOnException.exception(), + statement, + whitelistedException); } else { return statement; } } + @Nullable + private static Class<? extends Throwable> getExpectedException(Description description) { + Test test = description.getAnnotation(Test.class); + if (test.expected() != Test.None.class) { + return test.expected(); + } + + return null; + } + /** Retries a test in case of a failure. */ private static class RetryOnFailureStatement extends Statement { @@ -88,8 +94,13 @@ public class RetryRule implements TestRule { private int currentRun; private final Statement statement; + @Nullable private final Class<? extends Throwable> expectedException; - private RetryOnFailureStatement(int timesOnFailure, Statement statement) { + private RetryOnFailureStatement( + int timesOnFailure, + Statement statement, + @Nullable Class<? extends Throwable> expectedException) { + this.expectedException = expectedException; if (timesOnFailure < 0) { throw new IllegalArgumentException("Negatives number of retries on failure"); } @@ -109,6 +120,11 @@ public class RetryRule implements TestRule { statement.evaluate(); break; // success } catch (Throwable t) { + if (expectedException != null + && expectedException.isAssignableFrom(t.getClass())) { + throw t; + } + LOG.warn( String.format( "Test run failed (%d/%d).", currentRun, timesOnFailure + 1), @@ -129,13 +145,15 @@ public class RetryRule implements TestRule { private final Class<? extends Throwable> exceptionClass; private final int timesOnFailure; private final Statement statement; + @Nullable private final Class<? extends Throwable> expectedException; private int currentRun; private RetryOnExceptionStatement( int timesOnFailure, Class<? extends Throwable> exceptionClass, - Statement statement) { + Statement statement, + @Nullable Class<? extends Throwable> expectedException) { if (timesOnFailure < 0) { throw new IllegalArgumentException("Negatives number of retries on failure"); } @@ -146,6 +164,7 @@ public class RetryRule implements TestRule { this.exceptionClass = (exceptionClass); this.timesOnFailure = timesOnFailure; this.statement = statement; + this.expectedException = expectedException; } /** @@ -160,6 +179,11 @@ public class RetryRule implements TestRule { statement.evaluate(); break; // success } catch (Throwable t) { + if (expectedException != null + && expectedException.isAssignableFrom(t.getClass())) { + throw t; + } + LOG.warn( String.format( "Test run failed (%d/%d).", currentRun, timesOnFailure + 1), diff --git a/flink-test-utils-parent/flink-test-utils-junit/src/test/java/org/apache/flink/testutils/junit/RetryRuleTest.java b/flink-test-utils-parent/flink-test-utils-junit/src/test/java/org/apache/flink/testutils/junit/RetryRuleTest.java new file mode 100644 index 0000000..e2e3401 --- /dev/null +++ b/flink-test-utils-parent/flink-test-utils-junit/src/test/java/org/apache/flink/testutils/junit/RetryRuleTest.java @@ -0,0 +1,88 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.testutils.junit; + +import org.apache.flink.util.TestLogger; + +import org.junit.Assert; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +/** Tests for the {@link RetryRule}. */ +public class RetryRuleTest extends TestLogger { + + @Test + public void testExpectedExceptionIgnored() throws Throwable { + final RetryRule retryRule = new RetryRule(); + + final Description testDescription = + Description.createTestDescription( + TestClassWithTestExpectingRuntimeException.class, + "test", + TestClassWithTestExpectingRuntimeException.class + .getMethod("test") + .getAnnotations()); + + final TestStatement statement = new TestStatement(1); + + try { + retryRule.apply(statement, testDescription).evaluate(); + Assert.fail("Should have failed."); + } catch (RuntimeException expected) { + } + + assertThat(statement.getNumEvaluations(), is(1)); + } + + @Ignore // we don't want to actually this run as a test + private static class TestClassWithTestExpectingRuntimeException { + @RetryOnFailure(times = 2) + @Test(expected = RuntimeException.class) + public void test() {} + } + + private static class TestStatement extends Statement { + private final int numEvaluationsToFail; + + private int numEvaluations = 0; + + private TestStatement(int numEvaluationsToFail) { + this.numEvaluationsToFail = numEvaluationsToFail; + } + + @Override + public void evaluate() throws Throwable { + try { + if (numEvaluations < numEvaluationsToFail) { + throw new RuntimeException("test exception"); + } + } finally { + numEvaluations++; + } + } + + public int getNumEvaluations() { + return numEvaluations; + } + } +}
