From ea520306135d7626e8ca5d189cd1cfcce26a51ba Mon Sep 17 00:00:00 2001 From: XiangRongLin <41164160+XiangRongLin@users.noreply.github.com> Date: Sat, 16 Jan 2021 17:11:29 +0100 Subject: [PATCH 1/3] Add MockOnlyRule to allow skipping specific tests based on downloader --- .../java/org/schabi/newpipe/MockOnly.java | 18 +++++++ .../java/org/schabi/newpipe/MockOnlyRule.java | 48 +++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 extractor/src/test/java/org/schabi/newpipe/MockOnly.java create mode 100644 extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java diff --git a/extractor/src/test/java/org/schabi/newpipe/MockOnly.java b/extractor/src/test/java/org/schabi/newpipe/MockOnly.java new file mode 100644 index 000000000..2435fe2ab --- /dev/null +++ b/extractor/src/test/java/org/schabi/newpipe/MockOnly.java @@ -0,0 +1,18 @@ +package org.schabi.newpipe; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marker annotation to skip test if it not run with mocks. + * + * {@link MockOnlyRule} + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.METHOD, ElementType.TYPE}) +@Inherited +public @interface MockOnly { +} \ No newline at end of file diff --git a/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java b/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java new file mode 100644 index 000000000..78da16aaf --- /dev/null +++ b/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java @@ -0,0 +1,48 @@ +package org.schabi.newpipe; + +import org.junit.Assume; +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; +import org.schabi.newpipe.downloader.DownloaderType; + +import javax.annotation.Nonnull; + +/** + *

+ * Checks if the system variable "downloader" is set to "REAL" and skips the tests if it is. + * Otherwise execute the test. + *

+ * + *

+ * Use it by creating a public variable of this inside the test class and annotate it with + * {@link org.junit.Rule}. Then annotate the tests to be skipped with {@link MockOnly} + *

+ * + *

+ * Allows skipping unreliable or time sensitive tests in CI pipeline. + *

+ */ +public class MockOnlyRule implements TestRule { + + final String downloader = System.getProperty("downloader"); + + @Override + @Nonnull + public Statement apply(@Nonnull Statement base, @Nonnull Description description) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + final boolean hasAnnotation = description.getAnnotation(MockOnly.class) == null; + final boolean isMockDownloader = downloader == null || + !downloader.equalsIgnoreCase(DownloaderType.REAL.toString()); + Assume.assumeTrue( + "The test is not reliable against a website and thus skipped", + hasAnnotation && isMockDownloader + ); + + base.evaluate(); + } + }; + } +} From adf9d7d10fdc09bdafbe996e398436f6b53c61d4 Mon Sep 17 00:00:00 2001 From: XiangRongLin <41164160+XiangRongLin@users.noreply.github.com> Date: Fri, 19 Feb 2021 18:53:33 +0100 Subject: [PATCH 2/3] Add reason field to MockOnly This enforces developers to document why a test is skipped --- .../test/java/org/schabi/newpipe/MockOnly.java | 7 +++++++ .../java/org/schabi/newpipe/MockOnlyRule.java | 16 +++++++++------- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/extractor/src/test/java/org/schabi/newpipe/MockOnly.java b/extractor/src/test/java/org/schabi/newpipe/MockOnly.java index 2435fe2ab..69f8ac497 100644 --- a/extractor/src/test/java/org/schabi/newpipe/MockOnly.java +++ b/extractor/src/test/java/org/schabi/newpipe/MockOnly.java @@ -6,6 +6,8 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import javax.annotation.Nonnull; + /** * Marker annotation to skip test if it not run with mocks. * @@ -15,4 +17,9 @@ import java.lang.annotation.Target; @Target({ElementType.METHOD, ElementType.TYPE}) @Inherited public @interface MockOnly { + + /** + * Explanation why this test shold only be run with mocks and not against real websites + */ + @Nonnull String reason(); } \ No newline at end of file diff --git a/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java b/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java index 78da16aaf..bbdf3d739 100644 --- a/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java +++ b/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java @@ -33,13 +33,15 @@ public class MockOnlyRule implements TestRule { return new Statement() { @Override public void evaluate() throws Throwable { - final boolean hasAnnotation = description.getAnnotation(MockOnly.class) == null; - final boolean isMockDownloader = downloader == null || - !downloader.equalsIgnoreCase(DownloaderType.REAL.toString()); - Assume.assumeTrue( - "The test is not reliable against a website and thus skipped", - hasAnnotation && isMockDownloader - ); + final MockOnly annotation = description.getAnnotation(MockOnly.class); + if (annotation != null) { + final boolean isMockDownloader = downloader == null || + !downloader.equalsIgnoreCase(DownloaderType.REAL.toString()); + + Assume.assumeTrue("The test is not reliable against real website. Reason: " + + annotation.reason(), isMockDownloader); + } + base.evaluate(); } From e13e237392d0bbc106feddc1e876d7d45f0bc258 Mon Sep 17 00:00:00 2001 From: XiangRongLin <41164160+XiangRongLin@users.noreply.github.com> Date: Fri, 26 Feb 2021 17:47:22 +0100 Subject: [PATCH 3/3] Fix typo and reword some explanations --- .../java/org/schabi/newpipe/MockOnly.java | 4 ++-- .../java/org/schabi/newpipe/MockOnlyRule.java | 21 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/extractor/src/test/java/org/schabi/newpipe/MockOnly.java b/extractor/src/test/java/org/schabi/newpipe/MockOnly.java index 69f8ac497..30a39c703 100644 --- a/extractor/src/test/java/org/schabi/newpipe/MockOnly.java +++ b/extractor/src/test/java/org/schabi/newpipe/MockOnly.java @@ -9,7 +9,7 @@ import java.lang.annotation.Target; import javax.annotation.Nonnull; /** - * Marker annotation to skip test if it not run with mocks. + * Marker annotation to skip test in certain cases. * * {@link MockOnlyRule} */ @@ -19,7 +19,7 @@ import javax.annotation.Nonnull; public @interface MockOnly { /** - * Explanation why this test shold only be run with mocks and not against real websites + * Explanation why this test should be skipped */ @Nonnull String reason(); } \ No newline at end of file diff --git a/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java b/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java index bbdf3d739..76c739440 100644 --- a/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java +++ b/extractor/src/test/java/org/schabi/newpipe/MockOnlyRule.java @@ -9,19 +9,21 @@ import org.schabi.newpipe.downloader.DownloaderType; import javax.annotation.Nonnull; /** - *

- * Checks if the system variable "downloader" is set to "REAL" and skips the tests if it is. - * Otherwise execute the test. - *

- * - *

- * Use it by creating a public variable of this inside the test class and annotate it with - * {@link org.junit.Rule}. Then annotate the tests to be skipped with {@link MockOnly} - *

* *

* Allows skipping unreliable or time sensitive tests in CI pipeline. *

+ * + *

+ * Use it by creating a public variable of this inside the test class and annotate it with + * {@link org.junit.Rule}. Then annotate the specific tests to be skipped with {@link MockOnly} + *

+ * + *

+ * It works by checking if the system variable "downloader" is set to "REAL" and skips the tests if it is. + * Otherwise it executes the test. + *

+ */ public class MockOnlyRule implements TestRule { @@ -42,7 +44,6 @@ public class MockOnlyRule implements TestRule { + annotation.reason(), isMockDownloader); } - base.evaluate(); } };