From deb706635ec102fc508744868e0fa5d324ab7d3b Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 8 Apr 2022 14:53:28 +0100 Subject: Allow @TestCaseName to be for names that are compatible with CTS/AJUR Previously, @TestCaseName was disallowed altogether and the default format was set to {method}[{index}] which was compatible with CTS/AJUR. The intent was to prevent test authors from setting names which would break CTS/AJUR features such as retry. This change relaxes the restrictions to allow the use of @TestCaseName as long as the generated test names were compatible with CTS/AJUR, i.e. consisted of what could be a method name followed by an optional index in brackets. e.g. {method}_{0} would be allowed as long as the first parameter to the test contained characters that are allowed in an identifier. An invalid test name is detected as soon as it is created and will cause the test run to fail. Bug: 36541809 Test: Tried using @TestCaseName with valid and invalid patterns Change-Id: I1dbc99b6b181898b438997f3e6b6d79a60b212b6 --- README.google | 3 +++ .../naming/MacroSubstitutionNamingStrategy.java | 20 ++++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/README.google b/README.google index 47541ba..a1ef97e 100644 --- a/README.google +++ b/README.google @@ -18,3 +18,6 @@ Local Modifications: and prevent use of @TestCaseName. Ignore tests broken by the above change. 38419944 - Fix sharding on CTS. + 36541809 - Partially revert 36541809 to allow @TestCaseName to be + used as long as it generates a name that is compatible + with CTS and AJUR. diff --git a/src/main/java/junitparams/naming/MacroSubstitutionNamingStrategy.java b/src/main/java/junitparams/naming/MacroSubstitutionNamingStrategy.java index 1bdcafb..3386e51 100644 --- a/src/main/java/junitparams/naming/MacroSubstitutionNamingStrategy.java +++ b/src/main/java/junitparams/naming/MacroSubstitutionNamingStrategy.java @@ -23,6 +23,9 @@ public class MacroSubstitutionNamingStrategy implements TestCaseNamingStrategy { this.method = testMethod; } + // Android-added: allowable test names + private static final Pattern ALLOWABLE_TEST_NAMES = Pattern.compile("\\w+(\\[\\d+])?"); + @Override public String getTestCaseName(int parametersIndex, Object parameters) { TestCaseName testCaseName = method.getAnnotation(TestCaseName.class); @@ -30,6 +33,16 @@ public class MacroSubstitutionNamingStrategy implements TestCaseNamingStrategy { String template = getTemplate(testCaseName); String builtName = buildNameByTemplate(template, parametersIndex, parameters); + // Android-changed: CTS and AndroidJUnitRunner rely on specific format to test names, + // changing them will prevent CTS and AndroidJUnitRunner from working properly; + // see b/36541809 + if (!ALLOWABLE_TEST_NAMES.matcher(builtName).matches()) { + throw new IllegalStateException(String.format( + "@TestCaseName(\"%s\") not currently supported as it generated a test name of" + + " \"%s\" which will not work properly in CTS, must match \"%s\"", + template, builtName, ALLOWABLE_TEST_NAMES)); + } + if (builtName.trim().isEmpty()) { return buildNameByTemplate(DEFAULT_TEMPLATE, parametersIndex, parameters); } else { @@ -39,12 +52,7 @@ public class MacroSubstitutionNamingStrategy implements TestCaseNamingStrategy { private String getTemplate(TestCaseName testCaseName) { if (testCaseName != null) { - // Android-changed: CTS and AndroidJUnitRunner rely on specific format to test names, - // changing them will prevent CTS and AndroidJUnitRunner from working properly; - // see b/36541809 - throw new IllegalStateException( - "@TestCaseName not currently supported as it breaks running tests in CTS"); - // return testCaseName.value(); + return testCaseName.value(); } return DEFAULT_TEMPLATE; -- cgit v1.2.3