Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yaml parser: implement loadClasses flag #2688

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Current
New: Yaml parser: implement loadClasses flag (Dzmitry Sankouski)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for traceability, it would be good if you could please help do the following:

  1. Log a bug that explains the problem
  2. Refer to that bug id here so that the changeset information is available in the CHANGES.txt
  3. Add a reference to the issue in the description attribute of @Test test, so that we know that the test belongs to a specific defect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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)
Expand Down
45 changes: 44 additions & 1 deletion testng-core/src/main/java/org/testng/internal/Yaml.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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));
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we consider throwing org.testng.TestNGException instead with a proper error message that explains what went wrong ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

.getValueNode();
className = ((ScalarNode) valueNode).getValue();
} else {
className = ((ScalarNode) node).getValue();
}
return c.newInstance(className, loadClasses);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please use org.testng.internal.objects.InstanceCreator#newInstance(java.lang.reflect.Constructor<T>, java.lang.Object...) to create the instance ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

} catch (Exception e) {
throw new TestNGException("Failed to instantiate class", e);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
24 changes: 24 additions & 0 deletions testng-core/src/test/java/test/yaml/YamlTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@
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;
import org.testng.xml.internal.Parser;
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() {
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
name: My_Suite
tests:
- name: My_test
classes:
- this.class.does.not.Exists