Skip to content

Commit

Permalink
Android: Remove support for junit3 tests
Browse files Browse the repository at this point in the history 
We moved to junit4 long ago.

Bug: 40693265
Change-Id: I3be7bd0681fdb55bc95d63dafebedfcd7e646d33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5288616
Reviewed-by: Haiyang Pan <[email protected]>
Commit-Queue: Andrew Grieve <[email protected]>
Owners-Override: Andrew Grieve <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1260024}
  • Loading branch information
@agrieve
agrieve authored and Chromium LUCI CQ committed Feb 13, 2024
1 parent 01aa8ab commit 685060f
Show file tree
Hide file tree
Showing 13 changed files with 48 additions and 326 deletions.
57 changes: 0 additions & 57 deletions PRESUBMIT.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4373,60 +4373,6 @@ def _CheckAndroidCrLogUsage(input_api, output_api):
return results


def _CheckAndroidTestJUnitFrameworkImport(input_api, output_api):
"""Checks that junit.framework.* is no longer used."""
deprecated_junit_framework_pattern = input_api.re.compile(
r'^import junit\.framework\..*;', input_api.re.MULTILINE)
sources = lambda x: input_api.FilterSourceFile(
x, files_to_check=[r'.*\.java$'], files_to_skip=None)
errors = []
for f in input_api.AffectedFiles(file_filter=sources):
for line_num, line in f.ChangedContents():
if deprecated_junit_framework_pattern.search(line):
errors.append("%s:%d" % (f.LocalPath(), line_num))

results = []
if errors:
results.append(
output_api.PresubmitError(
'APIs from junit.framework.* are deprecated, please use JUnit4 framework'
'(org.junit.*) from //third_party/junit. Contact [email protected]'
' if you have any question.', errors))
return results


def _CheckAndroidTestJUnitInheritance(input_api, output_api):
"""Checks that if new Java test classes have inheritance.
Either the new test class is JUnit3 test or it is a JUnit4 test class
with a base class, either case is undesirable.
"""
class_declaration_pattern = input_api.re.compile(r'^public class \w*Test ')

sources = lambda x: input_api.FilterSourceFile(
x, files_to_check=[r'.*Test\.java$'], files_to_skip=None)
errors = []
for f in input_api.AffectedFiles(file_filter=sources):
if not f.OldContents():
class_declaration_start_flag = False
for line_num, line in f.ChangedContents():
if class_declaration_pattern.search(line):
class_declaration_start_flag = True
if class_declaration_start_flag and ' extends ' in line:
errors.append('%s:%d' % (f.LocalPath(), line_num))
if '{' in line:
class_declaration_start_flag = False

results = []
if errors:
results.append(
output_api.PresubmitPromptWarning(
'The newly created files include Test classes that inherits from base'
' class. Please do not use inheritance in JUnit4 tests or add new'
' JUnit3 tests. Contact [email protected] if you have any'
' questions.', errors))
return results


def _CheckAndroidTestAnnotationUsage(input_api, output_api):
"""Checks that android.test.suitebuilder.annotation.* is no longer used."""
deprecated_annotation_import_pattern = input_api.re.compile(
Expand Down Expand Up @@ -5326,9 +5272,6 @@ def ChecksAndroidSpecificOnUpload(input_api, output_api):
results.extend(_CheckAndroidDebuggableBuild(input_api, output_api))
results.extend(_CheckAndroidNewMdpiAssetLocation(input_api, output_api))
results.extend(_CheckAndroidToastUsage(input_api, output_api))
results.extend(_CheckAndroidTestJUnitInheritance(input_api, output_api))
results.extend(_CheckAndroidTestJUnitFrameworkImport(
input_api, output_api))
results.extend(_CheckAndroidTestAnnotationUsage(input_api, output_api))
results.extend(_CheckAndroidWebkitImports(input_api, output_api))
results.extend(_CheckAndroidXmlStyle(input_api, output_api, True))
Expand Down
90 changes: 0 additions & 90 deletions PRESUBMIT_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1470,96 +1470,6 @@ def testValidDepFromDownstream(self):
'Expected %d items, found %d: %s'
% (0, len(msgs), msgs))

class AndroidDeprecatedJUnitFrameworkTest(unittest.TestCase):
def testCheckAndroidTestJUnitFramework(self):
mock_input_api = MockInputApi()
mock_output_api = MockOutputApi()

mock_input_api.files = [
MockAffectedFile('LalaLand.java', [
'random stuff'
]),
MockAffectedFile('CorrectUsage.java', [
'import org.junit.ABC',
'import org.junit.XYZ;',
]),
MockAffectedFile('UsedDeprecatedJUnit.java', [
'import junit.framework.*;',
]),
MockAffectedFile('UsedDeprecatedJUnitAssert.java', [
'import junit.framework.Assert;',
]),
]
msgs = PRESUBMIT._CheckAndroidTestJUnitFrameworkImport(
mock_input_api, mock_output_api)
self.assertEqual(1, len(msgs),
'Expected %d items, found %d: %s'
% (1, len(msgs), msgs))
self.assertEqual(2, len(msgs[0].items),
'Expected %d items, found %d: %s'
% (2, len(msgs[0].items), msgs[0].items))
self.assertTrue('UsedDeprecatedJUnit.java:1' in msgs[0].items,
'UsedDeprecatedJUnit.java not found in errors')
self.assertTrue('UsedDeprecatedJUnitAssert.java:1'
in msgs[0].items,
'UsedDeprecatedJUnitAssert not found in errors')


class AndroidJUnitBaseClassTest(unittest.TestCase):
def testCheckAndroidTestJUnitBaseClass(self):
mock_input_api = MockInputApi()
mock_output_api = MockOutputApi()

mock_input_api.files = [
MockAffectedFile('LalaLand.java', [
'random stuff'
]),
MockAffectedFile('CorrectTest.java', [
'@RunWith(ABC.class);'
'public class CorrectTest {',
'}',
]),
MockAffectedFile('HistoricallyIncorrectTest.java', [
'public class Test extends BaseCaseA {',
'}',
], old_contents=[
'public class Test extends BaseCaseB {',
'}',
]),
MockAffectedFile('CorrectTestWithInterface.java', [
'@RunWith(ABC.class);'
'public class CorrectTest implement Interface {',
'}',
]),
MockAffectedFile('IncorrectTest.java', [
'public class IncorrectTest extends TestCase {',
'}',
]),
MockAffectedFile('IncorrectWithInterfaceTest.java', [
'public class Test implements X extends BaseClass {',
'}',
]),
MockAffectedFile('IncorrectMultiLineTest.java', [
'public class Test implements X, Y, Z',
' extends TestBase {',
'}',
]),
]
msgs = PRESUBMIT._CheckAndroidTestJUnitInheritance(
mock_input_api, mock_output_api)
self.assertEqual(1, len(msgs),
'Expected %d items, found %d: %s'
% (1, len(msgs), msgs))
self.assertEqual(3, len(msgs[0].items),
'Expected %d items, found %d: %s'
% (3, len(msgs[0].items), msgs[0].items))
self.assertTrue('IncorrectTest.java:1' in msgs[0].items,
'IncorrectTest not found in errors')
self.assertTrue('IncorrectWithInterfaceTest.java:1'
in msgs[0].items,
'IncorrectWithInterfaceTest not found in errors')
self.assertTrue('IncorrectMultiLineTest.java:2' in msgs[0].items,
'IncorrectMultiLineTest not found in errors')

class AndroidDebuggableBuildTest(unittest.TestCase):

Expand Down
65 changes: 6 additions & 59 deletions base/test/android/javatests/src/org/chromium/base/test/BaseChromiumAndroidJUnitRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@
import android.content.Context;
import android.content.ContextWrapper;
import android.content.SharedPreferences;
import android.content.pm.InstrumentationInfo;
import android.content.pm.PackageManager;
import android.content.pm.PackageManager.NameNotFoundException;
import android.os.Build;
import android.os.Build.VERSION;
import android.os.Bundle;
Expand Down Expand Up @@ -76,35 +73,11 @@
* <instrumentation>
*/
public class BaseChromiumAndroidJUnitRunner extends AndroidJUnitRunner {
private static final String LIST_ALL_TESTS_FLAG =
"org.chromium.base.test.BaseChromiumAndroidJUnitRunner.TestList";
private static final String LIST_TESTS_PACKAGE_FLAG =
"org.chromium.base.test.BaseChromiumAndroidJUnitRunner.TestListPackage";
private static final String IS_UNIT_TEST_FLAG =
"org.chromium.base.test.BaseChromiumAndroidJUnitRunner.IsUnitTest";
private static final String EXTRA_CLANG_COVERAGE_DEVICE_FILE =
"org.chromium.base.test.BaseChromiumAndroidJUnitRunner.ClangCoverageDeviceFile";

/**
* This flag is supported by AndroidJUnitRunner.
*
* See the following page for detail
* https://developer.android.com/reference/android/support/test/runner/AndroidJUnitRunner.html
*/
private static final String ARGUMENT_TEST_PACKAGE = "package";

/**
* The following arguments are corresponding to AndroidJUnitRunner command line arguments.
* `annotation`: run with only the argument annotation
* `notAnnotation`: run all tests except the ones with argument annotation
* `log`: run in log only mode, do not execute tests
*
* For more detail, please check
* https://developer.android.com/reference/android/support/test/runner/AndroidJUnitRunner.html
*/
private static final String ARGUMENT_ANNOTATION = "annotation";

private static final String ARGUMENT_NOT_ANNOTATION = "notAnnotation";
private static final String ARGUMENT_LOG_ONLY = "log";

private static final String TAG = "BaseJUnitRunner";
Expand All @@ -121,6 +94,7 @@ public class BaseChromiumAndroidJUnitRunner extends AndroidJUnitRunner {
private static final long FINISH_APP_TASKS_POLL_INTERVAL_MS = 100;

static InMemorySharedPreferencesContext sInMemorySharedPreferencesContext;
private static boolean sTestListMode;

static {
CommandLineInitUtil.setFilenameOverrideForTesting(CommandLineFlags.getTestCmdLineFile());
Expand Down Expand Up @@ -163,6 +137,7 @@ public void onCreate(Bundle arguments) {
if (arguments == null) {
arguments = new Bundle();
}
sTestListMode = "true".equals(arguments.getString(ARGUMENT_LOG_ONLY));
// Do not finish activities between tests so that batched tests can start
// an activity in @BeforeClass and have it live until @AfterClass.
arguments.putString("waitForActivitiesToComplete", "false");
Expand All @@ -184,7 +159,7 @@ public void onStart() {
LibraryLoader.setBrowserProcessStartupBlockedForTesting();
}

if (shouldListTests()) {
if (sTestListMode) {
Log.w(
TAG,
String.format(
Expand Down Expand Up @@ -243,50 +218,23 @@ public void waitForIdleSync() {
}
}

// TODO(yolandyan): Move this to test harness side once this class gets removed
private void addTestListPackage(Bundle bundle) {
PackageManager pm = getContext().getPackageManager();
InstrumentationInfo info;
try {
info = pm.getInstrumentationInfo(getComponentName(), PackageManager.GET_META_DATA);
} catch (NameNotFoundException e) {
Log.e(TAG, String.format("Could not find component %s", getComponentName()));
throw new RuntimeException(e);
}
Bundle metaDataBundle = info.metaData;
if (metaDataBundle != null && metaDataBundle.getString(LIST_TESTS_PACKAGE_FLAG) != null) {
bundle.putString(
ARGUMENT_TEST_PACKAGE, metaDataBundle.getString(LIST_TESTS_PACKAGE_FLAG));
}
}

private void listTests() {
Bundle results = new Bundle();
TestListInstrumentationRunListener listener = new TestListInstrumentationRunListener();
try {
TestExecutor.Builder executorBuilder = new TestExecutor.Builder(this);
executorBuilder.addRunListener(listener);
Bundle junit3Arguments = new Bundle(InstrumentationRegistry.getArguments());
junit3Arguments.putString(ARGUMENT_NOT_ANNOTATION, "org.junit.runner.RunWith");
addTestListPackage(junit3Arguments);
Request listJUnit3TestRequest = createListTestRequest(junit3Arguments);
results = executorBuilder.build().execute(listJUnit3TestRequest);

Bundle junit4Arguments = new Bundle(InstrumentationRegistry.getArguments());
junit4Arguments.putString(ARGUMENT_ANNOTATION, "org.junit.runner.RunWith");
addTestListPackage(junit4Arguments);

// Do not use Log runner from android test support.
//
// Test logging and execution skipping is handled by BaseJUnit4ClassRunner,
// having ARGUMENT_LOG_ONLY in argument bundle here causes AndroidJUnitRunner
// to use its own log-only class runner instead of BaseJUnit4ClassRunner.
Bundle junit4Arguments = new Bundle(InstrumentationRegistry.getArguments());
junit4Arguments.remove(ARGUMENT_LOG_ONLY);

Request listJUnit4TestRequest = createListTestRequest(junit4Arguments);
results.putAll(executorBuilder.build().execute(listJUnit4TestRequest));
listener.saveTestsToJson(
InstrumentationRegistry.getArguments().getString(LIST_ALL_TESTS_FLAG));
} catch (IOException | RuntimeException e) {
String msg = "Fatal exception when running tests";
Log.e(TAG, msg, e);
Expand Down Expand Up @@ -328,8 +276,7 @@ private Request createListTestRequest(Bundle arguments) {
}

static boolean shouldListTests() {
Bundle arguments = InstrumentationRegistry.getArguments();
return arguments != null && arguments.getString(LIST_ALL_TESTS_FLAG) != null;
return sTestListMode;
}

/**
Expand Down Expand Up @@ -557,7 +504,7 @@ private static boolean isTestMethod(Method m) {

@Override
public void finish(int resultCode, Bundle results) {
if (shouldListTests()) {
if (sTestListMode) {
super.finish(resultCode, results);
return;
}
Expand Down
28 changes: 18 additions & 10 deletions ...test/android/javatests/src/org/chromium/base/test/TestListInstrumentationRunListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@

package org.chromium.base.test;

import android.os.Bundle;

import androidx.test.InstrumentationRegistry;
import androidx.test.internal.runner.listener.InstrumentationRunListener;

import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.junit.runner.Description;
import org.junit.runner.Result;
import org.junit.runner.notification.Failure;
import org.junit.runners.model.InitializationError;

Expand All @@ -18,6 +22,7 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintStream;
import java.io.Writer;
import java.lang.annotation.Annotation;
import java.lang.reflect.Array;
Expand All @@ -26,17 +31,17 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

/** A RunListener that list out all the test information into a json file. */
public class TestListInstrumentationRunListener extends InstrumentationRunListener {
private static final String TAG = "TestListRunListener";
private static final String LIST_ALL_TESTS_FLAG =
"org.chromium.base.test.BaseChromiumAndroidJUnitRunner.TestList";

private static final Set<String> SKIP_METHODS =
new HashSet<>(
Arrays.asList(
new String[] {"toString", "hashCode", "annotationType", "equals"}));
Set.of("toString", "hashCode", "annotationType", "equals");

private final Map<Class<?>, JSONObject> mTestClassJsonMap = new HashMap<>();
private Failure mFirstFailure;
Expand Down Expand Up @@ -82,11 +87,8 @@ public void testStarted(Description desc) throws Exception {
+ desc.getTestClass());
}

/**
* Create a JSONArray with all the test class JSONObjects and save it to
* listed output path.
*/
public void saveTestsToJson(String outputPath) throws IOException {
/** Create a JSONArray with all the test class JSONObjects and save it to listed output path. */
private void saveTestsToJson(String outputPath) {
if (mFirstFailure != null) {
throw new RuntimeException(
"Failed on " + mFirstFailure.getDescription(), mFirstFailure.getException());
Expand All @@ -97,7 +99,7 @@ public void saveTestsToJson(String outputPath) throws IOException {
writer.write(allTestClassesJSON.toString());
} catch (IOException e) {
Log.e(TAG, "failed to write json to file", e);
throw e;
throw new RuntimeException(e);
}
}

Expand Down Expand Up @@ -186,4 +188,10 @@ private static Object asJSON(Object obj)
}
}
}

@Override
public void instrumentationRunFinished(
PrintStream streamResult, Bundle resultBundle, Result junitResults) {
saveTestsToJson(InstrumentationRegistry.getArguments().getString(LIST_ALL_TESTS_FLAG));
}
}
Loading

0 comments on commit 685060f

Please sign in to comment.