Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d2c0afa6 authored by John Wu's avatar John Wu
Browse files

Make RAVENWOOD_RUN_DISABLED_TESTS easier to understand

When the environment variable RAVENWOOD_RUN_DISABLED_TESTS is set

1. The logic for disabling/enabling tests is inverted; that is, tests
   that were originally enabled will be skipped, and tests that were
   originally disabled will run.

2. The regex pattern set in RAVENWOOD_REALLY_DISABLED can be used to
   "really" skip a test, even when RAVENWOOD_RUN_DISABLED_TESTS is
   enabled.

3. The existing behavior to "invert" the test result is removed. After
   this change, the test results will always be accurate, regardless of
   the flag used.

We also take the opportunity to remove the "private" APIs in
RavenwoodRule, and move tests that require poking into Ravenwood
internals to RavenwoodCoreTest.

Bug: 292141694
Flag: EXEMPT host side change only
Test: f/b/r/scripts/run-ravenwood-tests.sh
Change-Id: I8a8c2ee029d70e095da18e37a8487c2db7ae00dc
parent 2d6ae039
Loading
Loading
Loading
Loading
+3 −39
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ package android.platform.test.ravenwood;
import static com.android.ravenwood.common.RavenwoodCommonUtils.RAVENWOOD_VERBOSE_LOGGING;
import static com.android.ravenwood.common.RavenwoodCommonUtils.ensureIsPublicVoidMethod;

import static org.junit.Assert.fail;
import static org.junit.Assume.assumeTrue;

import android.annotation.NonNull;
@@ -229,13 +228,10 @@ public final class RavenwoodAwareTestRunner extends RavenwoodAwareTestRunnerBase
            s.evaluate();
            onAfter(description, scope, order, null);
        } catch (Throwable t) {
            var shouldReportFailure = RavenwoodCommonUtils.runIgnoringException(
                    () -> onAfter(description, scope, order, t));
            if (shouldReportFailure == null || shouldReportFailure) {
            RavenwoodCommonUtils.runIgnoringException(() -> onAfter(description, scope, order, t));
            throw t;
        }
    }
    }

    /**
     * A runner that simply skips a class. It still has to support {@link Filterable}
@@ -306,10 +302,8 @@ public final class RavenwoodAwareTestRunner extends RavenwoodAwareTestRunnerBase

    /**
     * Called after a test / class.
     *
     * Return false if the exception should be ignored.
     */
    private boolean onAfter(Description description, Scope scope, Order order, Throwable th) {
    private void onAfter(Description description, Scope scope, Order order, Throwable th) {
        if (RAVENWOOD_VERBOSE_LOGGING) {
            Log.v(TAG, "onAfter: description=" + description + ", " + scope + ", " + order + ", "
                    + th);
@@ -321,36 +315,6 @@ public final class RavenwoodAwareTestRunner extends RavenwoodAwareTestRunnerBase
            // End of a test method.
            mState.exitTestMethod(description);
        }

        // If RUN_DISABLED_TESTS is set, and the method did _not_ throw, make it an error.
        if (RavenwoodRule.private$ravenwood().isRunningDisabledTests()
                && scope == Scope.Instance && order == Order.Outer) {

            boolean isTestEnabled = RavenwoodEnablementChecker.shouldEnableOnRavenwood(
                    description, false);
            if (th == null) {
                // Test passed. Is the test method supposed to be enabled?
                if (isTestEnabled) {
                    // Enabled and didn't throw, okay.
                    return true;
                } else {
                    // Disabled and didn't throw. We should report it.
                    fail("Test wasn't included under Ravenwood, but it actually "
                            + "passed under Ravenwood; consider updating annotations");
                    return true; // unreachable.
                }
            } else {
                // Test failed.
                if (isTestEnabled) {
                    // Enabled but failed. We should throw the exception.
                    return true;
                } else {
                    // Disabled and failed. Expected. Don't throw.
                    return false;
                }
            }
        }
        return true;
    }

    /**
+72 −23
Original line number Diff line number Diff line
@@ -15,6 +15,8 @@
 */
package android.platform.test.ravenwood;

import static com.android.ravenwood.common.RavenwoodCommonUtils.log;

import android.annotation.NonNull;
import android.annotation.Nullable;
import android.platform.test.annotations.DisabledOnRavenwood;
@@ -22,10 +24,64 @@ import android.platform.test.annotations.EnabledOnRavenwood;

import org.junit.runner.Description;

import java.util.Objects;
import java.util.regex.Pattern;

/**
 * Calculates which tests need to be executed on Ravenwood.
 */
public class RavenwoodEnablementChecker {
    private static final String TAG = com.android.ravenwood.common.RavenwoodCommonUtils.TAG;

    /**
     * When this flag is enabled, all disabled tests will run, and all enabled tests will
     * be skipped to detect cases where a test is able to pass despite being marked as
     * {@link DisabledOnRavenwood}.
     *
     * This is typically helpful for internal maintainers discovering tests that had previously
     * been ignored, but now have enough Ravenwood-supported functionality to be enabled.
     *
     * RavenwoodCoreTest modifies it, so not final.
     */
    public static volatile boolean RUN_DISABLED_TESTS = "1".equals(
            System.getenv("RAVENWOOD_RUN_DISABLED_TESTS"));

    /**
     * When using RAVENWOOD_RUN_DISABLED_TESTS, you may still want to skip certain tests,
     * for example because the test would crash the JVM.
     *
     * This regex defines the tests that should still be disabled even if
     * RAVENWOOD_RUN_DISABLED_TESTS is set.
     *
     * Before running each test class and method, we check if this pattern can be found in
     * the full test name (either [class full name], or [class full name] + "#" + [method name]),
     * and if so, we skip it.
     *
     * For example, if you want to skip an entire test class, use:
     * RAVENWOOD_REALLY_DISABLE='\.CustomTileDefaultsRepositoryTest$'
     *
     * For example, if you want to skip a test method, use:
     * RAVENWOOD_REALLY_DISABLE='\.CustomTileDefaultsRepositoryTest#testSimple$'
     *
     * To ignore multiple classes, use (...|...), for example:
     * RAVENWOOD_REALLY_DISABLE='\.(ClassA|ClassB)$'
     *
     * Because we use a regex-find, setting "." would disable all tests.
     *
     * RavenwoodCoreTest modifies it, so not final.
     */
    public static volatile Pattern REALLY_DISABLED_PATTERN = Pattern.compile(
            Objects.requireNonNullElse(System.getenv("RAVENWOOD_REALLY_DISABLED"), ""));

    static {
        if (RUN_DISABLED_TESTS) {
            log(TAG, "$RAVENWOOD_RUN_DISABLED_TESTS enabled: running only disabled tests");
            if (!REALLY_DISABLED_PATTERN.pattern().isEmpty()) {
                log(TAG, "$RAVENWOOD_REALLY_DISABLED=" + REALLY_DISABLED_PATTERN.pattern());
            }
        }
    }

    private RavenwoodEnablementChecker() {
    }

@@ -38,23 +94,20 @@ public class RavenwoodEnablementChecker {
     * an {@link DisabledOnRavenwood} annotation.
     */
    public static boolean shouldEnableOnRavenwood(Description description,
            boolean takeIntoAccountRunDisabledTestsFlag) {
            boolean checkRunDisabledTestsFlag) {
        // First, consult any method-level annotations
        if (description.isTest()) {
            Boolean result = null;

            // Stopgap for http://g/ravenwood/EPAD-N5ntxM
            if (description.getMethodName().endsWith("$noRavenwood")) {
                result = false;
            } else if (description.getAnnotation(EnabledOnRavenwood.class) != null) {
            if (description.getAnnotation(EnabledOnRavenwood.class) != null) {
                result = true;
            } else if (description.getAnnotation(DisabledOnRavenwood.class) != null) {
                result = false;
            }
            if (result != null) {
                if (takeIntoAccountRunDisabledTestsFlag
                        && RavenwoodRule.private$ravenwood().isRunningDisabledTests()) {
                    result = !shouldStillIgnoreInProbeIgnoreMode(
                if (checkRunDisabledTestsFlag && RUN_DISABLED_TESTS) {
                    // Invert the result + check the really disable pattern
                    result = !result && !shouldReallyDisableTest(
                            description.getTestClass(), description.getMethodName());
                }
            }
@@ -65,43 +118,39 @@ public class RavenwoodEnablementChecker {

        // Otherwise, consult any class-level annotations
        return shouldRunClassOnRavenwood(description.getTestClass(),
                takeIntoAccountRunDisabledTestsFlag);
                checkRunDisabledTestsFlag);
    }

    public static boolean shouldRunClassOnRavenwood(@NonNull Class<?> testClass,
            boolean takeIntoAccountRunDisabledTestsFlag) {
            boolean checkRunDisabledTestsFlag) {
        boolean result = true;
        if (testClass.getAnnotation(EnabledOnRavenwood.class) != null) {
            result = true;
        } else if (testClass.getAnnotation(DisabledOnRavenwood.class) != null) {
            result = false;
        }
        if (!result) {
            if (takeIntoAccountRunDisabledTestsFlag
                    && RavenwoodRule.private$ravenwood().isRunningDisabledTests()) {
                result = !shouldStillIgnoreInProbeIgnoreMode(testClass, null);
            }
        if (checkRunDisabledTestsFlag && RUN_DISABLED_TESTS) {
            // Invert the result + check the really disable pattern
            result = !result && !shouldReallyDisableTest(testClass, null);
        }
        return result;
    }

    /**
     * Check if a test should _still_ disabled even if {@code RUN_DISABLED_TESTS}
     * is true, using {@code REALLY_DISABLED_PATTERN}.
     * Check if a test should _still_ disabled even if {@link #RUN_DISABLED_TESTS}
     * is true, using {@link #REALLY_DISABLED_PATTERN}.
     *
     * This only works on tests, not on classes.
     */
    static boolean shouldStillIgnoreInProbeIgnoreMode(
            @NonNull Class<?> testClass, @Nullable String methodName) {
        if (RavenwoodRule.private$ravenwood().getReallyDisabledPattern().pattern().isEmpty()) {
    static boolean shouldReallyDisableTest(@NonNull Class<?> testClass,
            @Nullable String methodName) {
        if (REALLY_DISABLED_PATTERN.pattern().isEmpty()) {
            return false;
        }

        final var fullname = testClass.getName() + (methodName != null ? "#" + methodName : "");

        System.out.println("XXX=" + fullname);

        if (RavenwoodRule.private$ravenwood().getReallyDisabledPattern().matcher(fullname).find()) {
        if (REALLY_DISABLED_PATTERN.matcher(fullname).find()) {
            System.out.println("Still ignoring " + fullname);
            return true;
        }
+0 −102
Original line number Diff line number Diff line
@@ -16,15 +16,8 @@

package android.platform.test.ravenwood;

import static com.android.ravenwood.common.RavenwoodCommonUtils.log;

import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.Instrumentation;
import android.content.Context;
import android.platform.test.annotations.DisabledOnRavenwood;

import androidx.test.platform.app.InstrumentationRegistry;

import com.android.ravenwood.common.RavenwoodCommonUtils;

@@ -32,64 +25,12 @@ import org.junit.rules.TestRule;
import org.junit.runner.Description;
import org.junit.runners.model.Statement;

import java.util.Objects;
import java.util.regex.Pattern;

/**
 * Reach out to g/ravenwood if you need any features in it.
 */
public final class RavenwoodRule implements TestRule {
    private static final String TAG = com.android.ravenwood.common.RavenwoodCommonUtils.TAG;

    static final boolean IS_ON_RAVENWOOD = RavenwoodCommonUtils.isOnRavenwood();

    /**
     * When this flag is enabled, all tests will be unconditionally run on Ravenwood to detect
     * cases where a test is able to pass despite being marked as {@link DisabledOnRavenwood}.
     *
     * This is typically helpful for internal maintainers discovering tests that had previously
     * been ignored, but now have enough Ravenwood-supported functionality to be enabled.
     */
    private static final boolean RUN_DISABLED_TESTS = "1".equals(
            System.getenv("RAVENWOOD_RUN_DISABLED_TESTS"));

    /**
     * When using ENABLE_PROBE_IGNORED, you may still want to skip certain tests,
     * for example because the test would crash the JVM.
     *
     * This regex defines the tests that should still be disabled even if ENABLE_PROBE_IGNORED
     * is set.
     *
     * Before running each test class and method, we check if this pattern can be found in
     * the full test name (either [class full name], or [class full name] + "#" + [method name]),
     * and if so, we skip it.
     *
     * For example, if you want to skip an entire test class, use:
     * RAVENWOOD_REALLY_DISABLE='\.CustomTileDefaultsRepositoryTest$'
     *
     * For example, if you want to skip an entire test class, use:
     * RAVENWOOD_REALLY_DISABLE='\.CustomTileDefaultsRepositoryTest#testSimple$'
     *
     * To ignore multiple classes, use (...|...), for example:
     * RAVENWOOD_REALLY_DISABLE='\.(ClassA|ClassB)$'
     *
     * Because we use a regex-find, setting "." would disable all tests.
     */
    private static final Pattern REALLY_DISABLED_PATTERN = Pattern.compile(
            Objects.requireNonNullElse(System.getenv("RAVENWOOD_REALLY_DISABLED"), ""));

    private static final boolean HAS_REALLY_DISABLE_PATTERN =
            !REALLY_DISABLED_PATTERN.pattern().isEmpty();

    static {
        if (RUN_DISABLED_TESTS) {
            log(TAG, "$RAVENWOOD_RUN_DISABLED_TESTS enabled: force running all tests");
            if (HAS_REALLY_DISABLE_PATTERN) {
                log(TAG, "$RAVENWOOD_REALLY_DISABLED=" + REALLY_DISABLED_PATTERN.pattern());
            }
        }
    }

    final RavenwoodTestProperties mProperties = new RavenwoodTestProperties();

    public static class Builder {
@@ -211,47 +152,4 @@ public final class RavenwoodRule implements TestRule {
            throw new RuntimeException(e);
        }
    }

    // Below are internal to ravenwood. Don't use them from normal tests...

    public static class RavenwoodPrivate {
        private RavenwoodPrivate() {
        }

        private volatile Boolean mRunDisabledTestsOverride = null;

        private volatile Pattern mReallyDisabledPattern = null;

        public boolean isRunningDisabledTests() {
            if (mRunDisabledTestsOverride != null) {
                return mRunDisabledTestsOverride;
            }
            return RUN_DISABLED_TESTS;
        }

        public Pattern getReallyDisabledPattern() {
            if (mReallyDisabledPattern != null) {
                return mReallyDisabledPattern;
            }
            return REALLY_DISABLED_PATTERN;
        }

        public void overrideRunDisabledTest(boolean runDisabledTests,
                @Nullable String reallyDisabledPattern) {
            mRunDisabledTestsOverride = runDisabledTests;
            mReallyDisabledPattern =
                    reallyDisabledPattern == null ? null : Pattern.compile(reallyDisabledPattern);
        }

        public void resetRunDisabledTest() {
            mRunDisabledTestsOverride = null;
            mReallyDisabledPattern = null;
        }
    }

    private static final RavenwoodPrivate sRavenwoodPrivate = new  RavenwoodPrivate();

    public static RavenwoodPrivate private$ravenwood() {
        return sRavenwoodPrivate;
    }
}
+9 −16
Original line number Diff line number Diff line
@@ -15,18 +15,21 @@
 */
package com.android.ravenwoodtest.bivalenttest.ravenizer;

import static org.junit.Assert.assertFalse;

import android.platform.test.annotations.DisabledOnRavenwood;
import android.platform.test.ravenwood.RavenwoodRule;
import android.util.Log;

import androidx.test.ext.junit.runners.AndroidJUnit4;

import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;

/**
 * This test shouldn't be executed on Ravenwood.
 */
@RunWith(AndroidJUnit4.class)
@DisabledOnRavenwood
public class RavenwoodImplicitClassRuleDeviceOnlyTest {
@@ -34,27 +37,17 @@ public class RavenwoodImplicitClassRuleDeviceOnlyTest {

    @BeforeClass
    public static void beforeClass() {
        // This method shouldn't be called -- unless RUN_DISABLED_TESTS is enabled.

        // If we're doing RUN_DISABLED_TESTS, don't throw here, because that'd confuse junit.
        if (!RavenwoodRule.private$ravenwood().isRunningDisabledTests()) {
            Assert.assertFalse(RavenwoodRule.isOnRavenwood());
        }
        // This method shouldn't be called
        assertFalse(RavenwoodRule.isOnRavenwood());
    }

    @Test
    public void testDeviceOnly() {
        Assert.assertFalse(RavenwoodRule.isOnRavenwood());
        assertFalse(RavenwoodRule.isOnRavenwood());
    }

    @AfterClass
    public static void afterClass() {
        if (RavenwoodRule.isOnRavenwood()) {
            Log.e(TAG, "Even @AfterClass shouldn't be executed!");

            if (!RavenwoodRule.private$ravenwood().isRunningDisabledTests()) {
                System.exit(1);
            }
        }
        assertFalse(RavenwoodRule.isOnRavenwood());
    }
}
+0 −87
Original line number Diff line number Diff line
/*
 * Copyright (C) 2024 The Android Open Source Project
 *
 * Licensed 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 com.android.ravenwoodtest.bivalenttest.ravenizer;

import static org.junit.Assert.fail;

import android.platform.test.annotations.DisabledOnRavenwood;
import android.platform.test.annotations.RavenwoodTestRunnerInitializing;
import android.platform.test.ravenwood.RavenwoodRule;
import android.util.Log;

import androidx.test.ext.junit.runners.AndroidJUnit4;

import org.junit.AfterClass;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;

/**
 * Test for "RAVENWOOD_RUN_DISABLED_TESTS". (with no "REALLY_DISABLED" set.)
 *
 * This test is only executed on Ravenwood.
 */
@RunWith(AndroidJUnit4.class)
public class RavenwoodRunDisabledTestsTest {
    private static final String TAG = "RavenwoodRunDisabledTestsTest";

    @Rule
    public ExpectedException mExpectedException = ExpectedException.none();

    private static final CallTracker sCallTracker = new CallTracker();

    @RavenwoodTestRunnerInitializing
    public static void ravenwoodRunnerInitializing() {
        RavenwoodRule.private$ravenwood().overrideRunDisabledTest(true, null);
    }

    @Test
    @DisabledOnRavenwood
    public void testDisabledTestGetsToRun() {
        if (!RavenwoodRule.isOnRavenwood()) {
            return;
        }
        sCallTracker.incrementMethodCallCount();

        fail("This test won't pass on Ravenwood.");
    }

    @Test
    @DisabledOnRavenwood
    public void testDisabledButPass() {
        if (!RavenwoodRule.isOnRavenwood()) {
            return;
        }
        sCallTracker.incrementMethodCallCount();

        // When a @DisabledOnRavenwood actually passed, the runner should make fail().
        mExpectedException.expectMessage("it actually passed under Ravenwood");
    }

    @AfterClass
    public static void afterClass() {
        if (!RavenwoodRule.isOnRavenwood()) {
            return;
        }
        Log.i(TAG, "afterClass called");

        sCallTracker.assertCallsOrDie(
                "testDisabledTestGetsToRun", 1,
                "testDisabledButPass", 1
        );
    }
}
Loading