From a1f2e3fa2cf74f36374f37987bcb23c92b635385 Mon Sep 17 00:00:00 2001 From: sankouski-dzmitry Date: Fri, 3 Dec 2021 19:28:39 +0300 Subject: [PATCH 1/2] Yaml parser: implement loadClasses flag Parser loadClasses flag controls, whether test classes will be loaded during suite parsing. Setting it to false allow to load yaml suite with non-existent classes, whereas true will fail parsing on test class loading. --- CHANGES.txt | 1 + .../main/java/org/testng/internal/Yaml.java | 45 ++++++++++++++++++- .../java/org/testng/internal/YamlParser.java | 2 +- .../src/test/java/test/yaml/YamlTest.java | 24 ++++++++++ .../yaml/suiteWithNonExistentTest.yaml | 5 +++ 5 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 testng-core/src/test/resources/yaml/suiteWithNonExistentTest.yaml diff --git a/CHANGES.txt b/CHANGES.txt index 9adb8da57e..1d86bb9fe8 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +New: Yaml parser: implement loadClasses flag (Dzmitry Sankouski) Fixed: GITHUB-2676: NPE is triggered when working with ITestObjectFactory (Krishnan Mahadevan) Fixed: GITHUB-2674: Run onTestSkipped for each value from data provider (Krishnan Mahadevan) Fixed: GITHUB-2672: Log real stacktrace when test times out. (cdalexndr) diff --git a/testng-core/src/main/java/org/testng/internal/Yaml.java b/testng-core/src/main/java/org/testng/internal/Yaml.java index 0db00ae70b..59dfa86c8b 100644 --- a/testng-core/src/main/java/org/testng/internal/Yaml.java +++ b/testng-core/src/main/java/org/testng/internal/Yaml.java @@ -7,6 +7,7 @@ import java.util.List; import java.util.Map; import java.util.function.Consumer; +import org.testng.TestNGException; import org.testng.util.Strings; import org.testng.xml.XmlClass; import org.testng.xml.XmlInclude; @@ -27,7 +28,8 @@ public final class Yaml { private Yaml() {} - public static XmlSuite parse(String filePath, InputStream is) throws FileNotFoundException { + public static XmlSuite parse(String filePath, InputStream is, boolean loadClasses) + throws FileNotFoundException { Constructor constructor = new TestNGConstructor(XmlSuite.class); { TypeDescription suiteDescription = new TypeDescription(XmlSuite.class); @@ -46,6 +48,9 @@ public static XmlSuite parse(String filePath, InputStream is) throws FileNotFoun constructor.addTypeDescription(testDescription); } + TypeDescription xmlClassDescription = new XmlClassTypeDescriptor(loadClasses); + constructor.addTypeDescription(xmlClassDescription); + org.yaml.snakeyaml.Yaml y = new org.yaml.snakeyaml.Yaml(constructor); if (is == null) { is = new FileInputStream(new File(filePath)); @@ -347,4 +352,42 @@ public Object construct(Node node) { } } } + + private static class XmlClassTypeDescriptor extends TypeDescription { + + private final boolean loadClasses; + + public XmlClassTypeDescriptor(boolean loadClasses) { + super(XmlClass.class); + this.loadClasses = loadClasses; + } + + @Override + public Object newInstance(Node node) { + String className; + + try { + java.lang.reflect.Constructor c = + XmlClass.class.getDeclaredConstructor(String.class, boolean.class); + c.setAccessible(true); + if (node instanceof MappingNode) { + Node valueNode = + ((MappingNode) node) + .getValue().stream() + .filter( + nodeTuple -> + ((ScalarNode) nodeTuple.getKeyNode()).getValue().equals("name")) + .findFirst() + .orElseThrow(RuntimeException::new) + .getValueNode(); + className = ((ScalarNode) valueNode).getValue(); + } else { + className = ((ScalarNode) node).getValue(); + } + return c.newInstance(className, loadClasses); + } catch (Exception e) { + throw new TestNGException("Failed to instantiate class", e); + } + } + } } diff --git a/testng-core/src/main/java/org/testng/internal/YamlParser.java b/testng-core/src/main/java/org/testng/internal/YamlParser.java index 617100ce57..a044978c26 100644 --- a/testng-core/src/main/java/org/testng/internal/YamlParser.java +++ b/testng-core/src/main/java/org/testng/internal/YamlParser.java @@ -13,7 +13,7 @@ public class YamlParser implements ISuiteParser { public XmlSuite parse(String filePath, InputStream is, boolean loadClasses) throws TestNGException { try { - return Yaml.parse(filePath, is); + return Yaml.parse(filePath, is, loadClasses); } catch (FileNotFoundException e) { throw new TestNGException(e); } diff --git a/testng-core/src/test/java/test/yaml/YamlTest.java b/testng-core/src/test/java/test/yaml/YamlTest.java index a99dec6fe0..2cae0896aa 100644 --- a/testng-core/src/test/java/test/yaml/YamlTest.java +++ b/testng-core/src/test/java/test/yaml/YamlTest.java @@ -14,6 +14,7 @@ import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import org.testng.internal.Yaml; +import org.testng.internal.YamlParser; import org.testng.reporters.Files; import org.testng.xml.SuiteXmlParser; import org.testng.xml.XmlSuite; @@ -21,6 +22,7 @@ import test.SimpleBaseTest; public class YamlTest extends SimpleBaseTest { + public static final String CLASS_NOT_FOUND_MESSAGE = "Cannot find class in classpath"; @DataProvider public Object[][] dp() { @@ -69,4 +71,26 @@ public void testXmlDependencyGroups() throws IOException { java.nio.file.Files.readAllBytes(Paths.get(expectedYamlFile)), StandardCharsets.UTF_8); assertThat(Yaml.toYaml(actualXmlSuite).toString()).isEqualToNormalizingNewlines(expectedYaml); } + + @Test + public void testLoadClassesFlag() throws IOException { + YamlParser yamlParser = new YamlParser(); + String yamlSuiteFile = "src/test/resources/yaml/suiteWithNonExistentTest.yaml"; + + try { + yamlParser.parse(yamlSuiteFile, new FileInputStream(yamlSuiteFile), false); + } catch (Throwable throwable) { + Throwable rootCause = getRootCause(throwable); + String rootCauseMessage = rootCause.getMessage(); + if (rootCauseMessage.contains(CLASS_NOT_FOUND_MESSAGE)) { + throw new AssertionError("TestNG shouldn't attempt to load test class", throwable); + } + + throw new AssertionError("Yaml parser failed to parse suite", throwable); + } + } + + private Throwable getRootCause(Throwable throwable) { + return throwable.getCause() != null ? getRootCause(throwable.getCause()) : throwable; + } } diff --git a/testng-core/src/test/resources/yaml/suiteWithNonExistentTest.yaml b/testng-core/src/test/resources/yaml/suiteWithNonExistentTest.yaml new file mode 100644 index 0000000000..429eb8fdd3 --- /dev/null +++ b/testng-core/src/test/resources/yaml/suiteWithNonExistentTest.yaml @@ -0,0 +1,5 @@ +name: My_Suite +tests: + - name: My_test + classes: + - this.class.does.not.Exists From 3c964b86619e5aaa9de63cc844d480ec7e477e02 Mon Sep 17 00:00:00 2001 From: sankouski-dzmitry Date: Tue, 7 Dec 2021 13:42:25 +0300 Subject: [PATCH 2/2] Review fixes - use InstanceCreator#newInstance() - use TestNG exception with detail exception message - create and link github issue --- CHANGES.txt | 2 +- testng-core/src/main/java/org/testng/internal/Yaml.java | 5 +++-- testng-core/src/test/java/test/yaml/YamlTest.java | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 1d86bb9fe8..185e4c2d01 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,5 @@ Current -New: Yaml parser: implement loadClasses flag (Dzmitry Sankouski) +Fixed: GITHUB-2689: Yaml parser: implement loadClasses flag (Dzmitry Sankouski) Fixed: GITHUB-2676: NPE is triggered when working with ITestObjectFactory (Krishnan Mahadevan) Fixed: GITHUB-2674: Run onTestSkipped for each value from data provider (Krishnan Mahadevan) Fixed: GITHUB-2672: Log real stacktrace when test times out. (cdalexndr) diff --git a/testng-core/src/main/java/org/testng/internal/Yaml.java b/testng-core/src/main/java/org/testng/internal/Yaml.java index 59dfa86c8b..cd737972e1 100644 --- a/testng-core/src/main/java/org/testng/internal/Yaml.java +++ b/testng-core/src/main/java/org/testng/internal/Yaml.java @@ -8,6 +8,7 @@ import java.util.Map; import java.util.function.Consumer; import org.testng.TestNGException; +import org.testng.internal.objects.InstanceCreator; import org.testng.util.Strings; import org.testng.xml.XmlClass; import org.testng.xml.XmlInclude; @@ -378,13 +379,13 @@ public Object newInstance(Node node) { nodeTuple -> ((ScalarNode) nodeTuple.getKeyNode()).getValue().equals("name")) .findFirst() - .orElseThrow(RuntimeException::new) + .orElseThrow(() -> new TestNGException("Node 'name' not found")) .getValueNode(); className = ((ScalarNode) valueNode).getValue(); } else { className = ((ScalarNode) node).getValue(); } - return c.newInstance(className, loadClasses); + return InstanceCreator.newInstance(c, className, loadClasses); } catch (Exception e) { throw new TestNGException("Failed to instantiate class", e); } diff --git a/testng-core/src/test/java/test/yaml/YamlTest.java b/testng-core/src/test/java/test/yaml/YamlTest.java index 2cae0896aa..85d639f373 100644 --- a/testng-core/src/test/java/test/yaml/YamlTest.java +++ b/testng-core/src/test/java/test/yaml/YamlTest.java @@ -72,7 +72,7 @@ public void testXmlDependencyGroups() throws IOException { assertThat(Yaml.toYaml(actualXmlSuite).toString()).isEqualToNormalizingNewlines(expectedYaml); } - @Test + @Test(description = "GITHUB-2689") public void testLoadClassesFlag() throws IOException { YamlParser yamlParser = new YamlParser(); String yamlSuiteFile = "src/test/resources/yaml/suiteWithNonExistentTest.yaml";