From dc0f4dbfee753143869b8c2db8491076483a9b4b Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Tue, 17 Jan 2023 13:25:36 +0100 Subject: [PATCH] [test] Refactor RuleTst to use only RuleTestDescriptor Only use RuleTestDescriptor instead of TestDescriptor. TestDescriptor can be removed then. Remove previously internalized methods or make them private. --- .../pmd/testframework/RuleTestRunner.java | 8 +- .../pmd/testframework/RuleTst.java | 119 +++++------------- .../pmd/testframework/RuleTstTest.java | 16 ++- 3 files changed, 48 insertions(+), 95 deletions(-) diff --git a/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTestRunner.java b/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTestRunner.java index 5e08bf3d3b..7f959ccb54 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTestRunner.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTestRunner.java @@ -111,10 +111,16 @@ public class RuleTestRunner extends ParentRunner { * @return a single statement which includes any rules, if present. */ private Statement ruleTestBlock(final TestDescriptor testCase) { + RuleTestDescriptor newTestCase = new RuleTestDescriptor(testCase.getNumberInDocument(), testCase.getRule()); + newTestCase.setLanguageVersion(testCase.getLanguageVersion()); + newTestCase.setCode(testCase.getCode()); + newTestCase.setDescription(testCase.getDescription()); + newTestCase.setDisabled(!testCase.isRegressionTest()); + newTestCase.recordExpectedViolations(testCase.getNumberOfProblemsExpected(), testCase.getExpectedLineNumbers(), testCase.getExpectedMessages()); Statement statement = new Statement() { @Override public void evaluate() { - instance.runTest(testCase); + instance.runTest(newTestCase); } }; statement = withBefores(statement); diff --git a/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java b/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java index c3be96ee03..7a7ae69291 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java @@ -12,13 +12,13 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.StringWriter; +import java.net.URI; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.junit.jupiter.api.DynamicTest; @@ -95,13 +95,9 @@ public abstract class RuleTst { } /** - * Run the rule on the given code, and check the expected number of - * violations. + * Run the rule on the given code, and check the expected number of violations. */ - @SuppressWarnings("unchecked") - @InternalApi - @Deprecated - public void runTest(TestDescriptor test) { + void runTest(RuleTestDescriptor test) { Rule rule = test.getRule(); // always reinitialize the rule, regardless of test.getReinitializeRule() (#3976 / #3302) @@ -138,11 +134,11 @@ public abstract class RuleTst { e.printStackTrace(); throw new RuntimeException('"' + test.getDescription() + "\" failed", e); } - if (test.getNumberOfProblemsExpected() != res) { + if (test.getExpectedProblems() != res) { printReport(test, report); } - assertEquals(test.getNumberOfProblemsExpected(), res, - '"' + test.getDescription() + "\" resulted in wrong number of failures,"); + assertEquals(test.getExpectedProblems(), res, + '"' + test.getDescription() + "\" resulted in wrong number of failures,"); assertMessages(report, test); assertLineNumbers(report, test); } finally { @@ -166,7 +162,7 @@ public abstract class RuleTst { } - private void assertMessages(Report report, TestDescriptor test) { + private void assertMessages(Report report, RuleTestDescriptor test) { if (report == null || test.getExpectedMessages().isEmpty()) { return; } @@ -189,7 +185,7 @@ public abstract class RuleTst { } } - private void assertLineNumbers(Report report, TestDescriptor test) { + private void assertLineNumbers(Report report, RuleTestDescriptor test) { if (report == null || test.getExpectedLineNumbers().isEmpty()) { return; } @@ -214,10 +210,10 @@ public abstract class RuleTst { } } - private void printReport(TestDescriptor test, Report report) { + private void printReport(RuleTestDescriptor test, Report report) { System.out.println("--------------------------------------------------------------"); System.out.println("Test Failure: " + test.getDescription()); - System.out.println(" -> Expected " + test.getNumberOfProblemsExpected() + " problem(s), " + report.getViolations().size() + System.out.println(" -> Expected " + test.getExpectedProblems() + " problem(s), " + report.getViolations().size() + " problem(s) found."); System.out.println(" -> Expected messages: " + test.getExpectedMessages()); System.out.println(" -> Expected line numbers: " + test.getExpectedLineNumbers()); @@ -235,7 +231,7 @@ public abstract class RuleTst { System.out.println("--------------------------------------------------------------"); } - private Report processUsingStringReader(TestDescriptor test, Rule rule) { + private Report processUsingStringReader(RuleTestDescriptor test, Rule rule) { return runTestFromString(test.getCode(), rule, test.getLanguageVersion()); } @@ -286,19 +282,11 @@ public abstract class RuleTst { } } - @InternalApi - @Deprecated - public Report runTestFromString(TestDescriptor test, Rule rule) { - return runTestFromString(test.getCode(), rule, test.getLanguageVersion(), test.isUseAuxClasspath()); - } - /** * getResourceAsStream tries to find the XML file in weird locations if the * ruleName includes the package, so we strip it here. */ - @InternalApi - @Deprecated - protected String getCleanRuleName(Rule rule) { + private String getCleanRuleName(Rule rule) { String fullClassName = rule.getClass().getName(); if (fullClassName.equals(rule.getName())) { // We got the full class name, so we'll use the stripped name @@ -310,19 +298,6 @@ public abstract class RuleTst { } } - /** - * Extract a set of tests from an XML file. The file should be - * ./xml/RuleName.xml relative to the test class. The format is defined in - * test-data.xsd. - */ - @InternalApi - @Deprecated - public TestDescriptor[] extractTestsFromXml(Rule rule) { - String testsFileName = getCleanRuleName(rule); - - return extractTestsFromXml(rule, testsFileName); - } - /** * Extract a set of tests from an XML file. The file should be * ./xml/RuleName.xml relative to the test class. The format is defined in @@ -330,35 +305,13 @@ public abstract class RuleTst { */ RuleTestCollection parseTestCollection(Rule rule) { String testsFileName = getCleanRuleName(rule); + return parseTestCollection(rule, testsFileName); + } + + private RuleTestCollection parseTestCollection(Rule rule, String testsFileName) { return parseTestXml(rule, testsFileName, "xml/"); } - @InternalApi - @Deprecated - public TestDescriptor[] extractTestsFromXml(Rule rule, String testsFileName) { - return extractTestsFromXml(rule, testsFileName, "xml/"); - } - - /** - * Extract a set of tests from an XML file with the given name. The file - * should be ./xml/[testsFileName].xml relative to the test class. The - * format is defined in test-data.xsd. - */ - @InternalApi - @Deprecated - public TestDescriptor[] extractTestsFromXml(Rule rule, String testsFileName, String baseDirectory) { - RuleTestCollection collection = parseTestXml(rule, testsFileName, baseDirectory); - return toLegacyArray(collection); - } - - private TestDescriptor[] toLegacyArray(RuleTestCollection collection) { - TestDescriptor[] result = new TestDescriptor[collection.getTests().size()]; - for (int i = 0; i < collection.getTests().size(); i++) { - result[i] = new TestDescriptor(collection.getTests().get(i), collection.getAbsoluteUriToTestXmlFile()); - } - return result; - } - /** * Extract a set of tests from an XML file with the given name. The file * should be ./xml/[testsFileName].xml relative to the test class. The @@ -392,7 +345,7 @@ public abstract class RuleTst { * defined in test-data.xsd. */ public void runTests(Rule rule) { - runTests(extractTestsFromXml(rule)); + runTests(parseTestCollection(rule)); } /** @@ -401,16 +354,11 @@ public abstract class RuleTst { * defined in test-data.xsd. */ public void runTests(Rule rule, String testsFileName) { - runTests(extractTestsFromXml(rule, testsFileName)); + runTests(parseTestCollection(rule, testsFileName)); } - /** - * Run a set of tests of a certain sourceType. - */ - @InternalApi - @Deprecated - public void runTests(TestDescriptor[] tests) { - for (TestDescriptor test : tests) { + private void runTests(RuleTestCollection tests) { + for (RuleTestDescriptor test : tests.getTests()) { runTest(test); } } @@ -421,34 +369,29 @@ public abstract class RuleTst { final List rules = new ArrayList<>(getRules()); rules.sort(Comparator.comparing(Rule::getName)); - List tests = new ArrayList<>(); + List tests = new ArrayList<>(); for (Rule r : rules) { RuleTestCollection ruleTests = parseTestCollection(r); RuleTestDescriptor focused = ruleTests.getFocusedTestOrNull(); for (RuleTestDescriptor t : ruleTests.getTests()) { - TestDescriptor td = new TestDescriptor(t, ruleTests.getAbsoluteUriToTestXmlFile()); if (focused != null && !focused.equals(t)) { - td.setRegressionTest(false); // disable it + t.setDisabled(true); // disable it } - tests.add(td); + tests.add(toDynamicTest(ruleTests, t)); } } - - return tests.stream().map(this::toDynamicTest).collect(Collectors.toList()); + return tests; } - private DynamicTest toDynamicTest(TestDescriptor testDescriptor) { - if (isIgnored(testDescriptor)) { - return DynamicTest.dynamicTest("[IGNORED] " + testDescriptor.getTestMethodName(), - testDescriptor.getTestSourceUri(), + private DynamicTest toDynamicTest(RuleTestCollection collection, RuleTestDescriptor testDescriptor) { + URI testSourceUri = URI.create(collection.getAbsoluteUriToTestXmlFile() + "?line=" + testDescriptor.getLineNumber()); + if (testDescriptor.isDisabled()) { + return DynamicTest.dynamicTest("[IGNORED] " + testDescriptor.getDescription(), + testSourceUri, () -> {}); } - return DynamicTest.dynamicTest(testDescriptor.getTestMethodName(), - testDescriptor.getTestSourceUri(), + return DynamicTest.dynamicTest(testDescriptor.getDescription(), + testSourceUri, () -> runTest(testDescriptor)); } - - private static boolean isIgnored(TestDescriptor testDescriptor) { - return TestDescriptor.inRegressionTestMode() && !testDescriptor.isRegressionTest(); - } } diff --git a/pmd-test/src/test/java/net/sourceforge/pmd/testframework/RuleTstTest.java b/pmd-test/src/test/java/net/sourceforge/pmd/testframework/RuleTstTest.java index ea8b0e5ed1..3d521ef0e4 100644 --- a/pmd-test/src/test/java/net/sourceforge/pmd/testframework/RuleTstTest.java +++ b/pmd-test/src/test/java/net/sourceforge/pmd/testframework/RuleTstTest.java @@ -11,6 +11,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.Arrays; +import java.util.Collections; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -23,8 +24,9 @@ import net.sourceforge.pmd.lang.document.TextRegion; import net.sourceforge.pmd.lang.rule.RuleTargetSelector; import net.sourceforge.pmd.test.lang.DummyLanguageModule; import net.sourceforge.pmd.test.lang.DummyLanguageModule.DummyRootNode; +import net.sourceforge.pmd.test.schema.RuleTestDescriptor; -public class RuleTstTest { +class RuleTstTest { private LanguageVersion dummyLanguage = DummyLanguageModule.getInstance().getDefaultVersion(); private Rule rule = mock(Rule.class); @@ -37,7 +39,7 @@ public class RuleTstTest { }; @Test - public void shouldCallStartAndEnd() { + void shouldCallStartAndEnd() { when(rule.getLanguage()).thenReturn(dummyLanguage.getLanguage()); when(rule.getName()).thenReturn("test rule"); when(rule.getTargetSelector()).thenReturn(RuleTargetSelector.forRootOnly()); @@ -57,7 +59,7 @@ public class RuleTstTest { } @Test - public void shouldAssertLinenumbersSorted() { + void shouldAssertLinenumbersSorted() { when(rule.getLanguage()).thenReturn(dummyLanguage.getLanguage()); when(rule.getName()).thenReturn("test rule"); when(rule.getMessage()).thenReturn("test rule"); @@ -77,9 +79,11 @@ public class RuleTstTest { return null; }).when(rule).apply(any(Node.class), Mockito.any(RuleContext.class)); - TestDescriptor testDescriptor = new TestDescriptor(code, "sample test", 2, rule, dummyLanguage); - testDescriptor.setReinitializeRule(false); - testDescriptor.setExpectedLineNumbers(Arrays.asList(1, 2)); + RuleTestDescriptor testDescriptor = new RuleTestDescriptor(0, rule); + testDescriptor.setLanguageVersion(dummyLanguage); + testDescriptor.setCode(code); + testDescriptor.setDescription("sample test"); + testDescriptor.recordExpectedViolations(2, Arrays.asList(1, 2), Collections.emptyList()); ruleTester.runTest(testDescriptor); }