-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
OpenTestReportGeneratingListenerTests is fixed on JDK 22-ea+27 and later #3594
Comments
I disabled the failing test in 15d94ad |
Marc, would it be possible to extract the relevant code and create a runnable test? The change (openjdk/jdk#16983) you mentioned made the processor to skip catalogs if both the public and system IDs are null. Is it the case that OpenTestReportGeneratingListenerTests and the custom catalog it uses tolerate null IDs, in other words, the test actually requires null IDs be processed with the custom catalog? In any case, a test would be nice so we have a test case. |
@JoeWang-Java Will do! |
The issue can be reproduced by running https://github.com/ota4j-team/open-test-reporting/blob/a70e19c273e2574a71dbb0090cfa7505bbe608f2/tooling/src/test/java/org/opentest4j/reporting/tooling/validator/DefaultValidatorTests.java#L38 on JDK 22-ea+27. @JoeWang-Java Is that sufficient for you or do you need a standalone reproducer? |
I don't see some dependencies, e.g. DefaultValidator. If that's not public, would it be possible to make a copy of the relevant portion? |
@JoeWang-Java Please follow these instructions to reproduce the issue:
This requires Gradle to be able to auto-detect a JDK 22 toolchain in version 22-ea+27 or later. If that doesn't work out-of-the-box, you can be explicit:
|
Hi Marc, happy New Year! I'm just back from the holidays to look at this. The reason I asked about a runnable test is that my time is allocated to items that can result in a quick resolution, esp. as we're approaching RDP2. I don't think I need to run your test to prove it as I trust your test result. A runnable/standalone test, standalone meaning having no dependency on your test environment, would be very helpful for creating a quick patch. The issue is obvious, it's just that I'd like to put a test in that represents (hopefully exactly) the use case found in your test so that the same mistake won't happen again. |
@JoeWang-Java I'll see what I can do to create a standalone reproducer. |
Thanks. And by the way, the change made in the JDK was to skip the custom catalog when both public and system IDs are null. I assume therefore the failing test passes null IDs to a catalog, or it could be said that it failed to check null values. |
@JoeWang-Java I've tried with your JDK build provided to me via @sormuras but the problem persists: https://scans.gradle.com/s/teupjnwamuksw/timeline?details=bxq5paipccw2s I've created a standalone reproducer: import org.w3c.dom.ls.LSInput;
import org.w3c.dom.ls.LSResourceResolver;
import org.xml.sax.SAXException;
import javax.xml.catalog.CatalogFeatures;
import javax.xml.transform.Source;
import javax.xml.transform.stream.StreamSource;
import javax.xml.validation.SchemaFactory;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.StringReader;
import java.util.Map;
import static java.util.Objects.requireNonNull;
import static javax.xml.XMLConstants.W3C_XML_SCHEMA_NS_URI;
import static javax.xml.catalog.CatalogManager.catalogResolver;
public class Reproducer {
private static final Map<String, String> SCHEMAS = Map.of(
"https://schemas.opentest4j.org/reporting/events/0.1.0", "events.xsd",
"https://schemas.opentest4j.org/reporting/core/0.1.0", "core.xsd"
);
public static void main(String[] args) throws Exception {
var xml = """
<events xmlns="https://schemas.opentest4j.org/reporting/events/0.1.0"/>
""";
validate(new StreamSource(new StringReader(xml)));
System.out.println("Successfully validated");
}
private static void validate(Source source) throws SAXException, IOException {
var schemaFactory = SchemaFactory.newInstance(W3C_XML_SCHEMA_NS_URI);
var validator = schemaFactory.newSchema().newValidator();
validator.setResourceResolver(createResourceResolver());
validator.validate(source);
}
private static LSResourceResolver createResourceResolver() {
return (type, namespaceURI, publicId, systemId, baseURI) -> {
if (namespaceURI != null) {
if (SCHEMAS.containsKey(namespaceURI)) {
CustomLSInputImpl input = new CustomLSInputImpl();
input.setPublicId(publicId);
var schema = SCHEMAS.get(namespaceURI);
input.setSystemId(requireNonNull(Reproducer.class.getResource(schema)).toExternalForm());
input.setBaseURI(baseURI);
var stream = Reproducer.class.getResourceAsStream(schema);
input.setCharacterStream(new InputStreamReader(requireNonNull(stream)));
return input;
}
}
if (systemId != null) {
var features = CatalogFeatures.builder()
.with(CatalogFeatures.Feature.RESOLVE, "continue")
.build();
var catalogResolver = catalogResolver(features);
return catalogResolver.resolveResource(type, namespaceURI, publicId, systemId, baseURI);
}
return null;
};
}
static class CustomLSInputImpl implements LSInput {
private Reader characterStream;
private InputStream byteStream;
private String stringData;
private String systemId;
private String publicId;
private String baseURI;
private String encoding;
private boolean certifiedText;
@Override
public Reader getCharacterStream() {
return characterStream;
}
@Override
public void setCharacterStream(Reader characterStream) {
this.characterStream = characterStream;
}
@Override
public InputStream getByteStream() {
return byteStream;
}
@Override
public void setByteStream(InputStream byteStream) {
this.byteStream = byteStream;
}
@Override
public String getStringData() {
return stringData;
}
@Override
public void setStringData(String stringData) {
this.stringData = stringData;
}
@Override
public String getSystemId() {
return systemId;
}
@Override
public void setSystemId(String systemId) {
this.systemId = systemId;
}
@Override
public String getPublicId() {
return publicId;
}
@Override
public void setPublicId(String publicId) {
this.publicId = publicId;
}
@Override
public String getBaseURI() {
return baseURI;
}
@Override
public void setBaseURI(String baseURI) {
this.baseURI = baseURI;
}
@Override
public String getEncoding() {
return encoding;
}
@Override
public void setEncoding(String encoding) {
this.encoding = encoding;
}
@Override
public boolean getCertifiedText() {
return certifiedText;
}
@Override
public void setCertifiedText(boolean certifiedText) {
this.certifiedText = certifiedText;
}
}
} It requires
Here's a zip archive with all files: catalog-reproducer.zip |
@JoeWang-Java Feel free to copy the above code or a simplified version thereof to the OpenJDK test suite, if you think that makes sense. |
Thanks Marc! I'll try to get a fix in before RDP2. |
A change in JDK 22-ea+27 is causing
OpenTestReportGeneratingListenerTests
to fail.Test passing on 22-ea+26: https://ge.junit.org/s/4nlyomk3crdoc/timeline?details=ikpytbbcborns&name=platform-tests:test
Test failing on 22-ea+27: https://ge.junit.org/s/sdadapber3wcm/timeline?details=ikpytbbcborns&name=platform-tests:test
The test uses a custom catalog and the used
DefaultValidator
from open-test-reporting uses a customLSResourceResolver
. Therefore, it seems likely that it's a change related to XML that was made in b27: https://bugs.openjdk.org/browse/JDK-8321406?jql=project%20%3D%20JDK%20AND%20component%20%3D%20xml%20AND%20%22Resolved%20In%20Build%22%20%3D%20b27%20ORDER%20BY%20cf%5B10006%5D%20ASC%2C%20created%20DESC. Maybe openjdk/jdk#16983?The XML file in question looks like this:
The text was updated successfully, but these errors were encountered: