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

OpenTestReportGeneratingListenerTests is fixed on JDK 22-ea+27 and later #3594

Open
marcphilipp opened this issue Dec 9, 2023 · 12 comments
Open
Assignees

Comments

@marcphilipp
Copy link
Member

marcphilipp commented Dec 9, 2023

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 custom LSResourceResolver. 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:

<?xml version="1.0" ?>
<e:events xmlns="https://schemas.opentest4j.org/reporting/core/0.1.0" xmlns:e="https://schemas.opentest4j.org/reporting/events/0.1.0" xmlns:java="https://schemas.opentest4j.org/reporting/java/0.1.0" xmlns:junit="https://schemas.junit.org/open-test-reporting" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="https://schemas.junit.org/open-test-reporting https://junit.org/junit5/schemas/open-test-reporting/junit-1.9.xsd">
<infrastructure><hostName>Marcs-M1-MBP.fritz.box</hostName><userName>marc</userName><operatingSystem>Mac OS X</operatingSystem><cpuCores>10</cpuCores><java:javaVersion>22-ea</java:javaVersion><java:fileEncoding>UTF-8</java:fileEncoding><java:heapSize max="1073741824"></java:heapSize></infrastructure>
<e:started id="1" name="dummy" time="2023-12-09T17:35:39.407811Z"><metadata><junit:uniqueId>[engine:dummy]</junit:uniqueId><junit:legacyReportingName>dummy</junit:legacyReportingName><junit:type>CONTAINER</junit:type></metadata></e:started>
<e:started id="2" name="display&lt;--&gt;Name 😎" parentId="1" time="2023-12-09T17:35:39.408094Z"><metadata><junit:uniqueId>[engine:dummy]/[test:failingTest]</junit:uniqueId><junit:legacyReportingName>display&lt;--&gt;Name 😎</junit:legacyReportingName><junit:type>TEST</junit:type></metadata></e:started>
<e:reported id="2" time="2023-12-09T17:35:39.411030Z"><attachments><data time="2023-12-09T18:35:39.410495"><entry key="key">value</entry></data></attachments></e:reported>
<e:finished id="2" time="2023-12-09T17:35:39.415289Z"><result status="FAILED"><java:throwable assertionError="true" type="org.opentest4j.AssertionFailedError"><![CDATA[org.opentest4j.AssertionFailedError: failure message
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1597)
]]></java:throwable></result></e:finished>
<e:finished id="1" time="2023-12-09T17:35:39.417640Z"><result status="SUCCESSFUL"></result></e:finished>
</e:events>
@marcphilipp
Copy link
Member Author

I disabled the failing test in 15d94ad

@JoeWang-Java
Copy link

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.

@marcphilipp
Copy link
Member Author

@JoeWang-Java Will do!

@marcphilipp
Copy link
Member Author

@JoeWang-Java
Copy link

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?

@marcphilipp
Copy link
Member Author

@JoeWang-Java DefaultValidator is in the same repo: https://github.com/ota4j-team/open-test-reporting/blob/main/tooling/src/main/java/org/opentest4j/reporting/tooling/validator/DefaultValidator.java

Please follow these instructions to reproduce the issue:

  1. Clone https://github.com/ota4j-team/open-test-reporting/
  2. Check out the marc/jdk22 branch
  3. Run ./gradlew :tooling:test --tests DefaultValidatorTests.validatesEventsXmlFile

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:

./gradlew :tooling:test --tests DefaultValidatorTests.validatesEventsXmlFile \
	-Porg.gradle.java.installations.auto-detect=false \
	-Porg.gradle.java.installations.paths=<JDK17_HOME>,<JDK22_HOME>

@JoeWang-Java
Copy link

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.

@marcphilipp marcphilipp self-assigned this Jan 7, 2024
@marcphilipp
Copy link
Member Author

@JoeWang-Java I'll see what I can do to create a standalone reproducer.

@JoeWang-Java
Copy link

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.

@marcphilipp
Copy link
Member Author

marcphilipp commented Jan 10, 2024

@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 events.xsd and core.xsd to be present in the same directory and can then be called via

$ jdk-23/fastdebug/bin/java --show-version Reproducer.java

java 23-internal 2024-09-17
Java(TM) SE Runtime Environment (fastdebug build 23-internal-2024-01-09-1826370.huizhe.wang.jdk)
Java HotSpot(TM) 64-Bit Server VM (fastdebug build 23-internal-2024-01-09-1826370.huizhe.wang.jdk, mixed mode, sharing)
Exception in thread "main" org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 72; cvc-elt.1.a: Cannot find the declaration of element 'events'.

$ java --show-version Reproducer.java

openjdk 21.0.1 2023-10-17 LTS
OpenJDK Runtime Environment (build 21.0.1+12-LTS)
OpenJDK 64-Bit Server VM (build 21.0.1+12-LTS, mixed mode, sharing)
Successfully validated

Here's a zip archive with all files: catalog-reproducer.zip

@marcphilipp
Copy link
Member Author

@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.

@JoeWang-Java
Copy link

Thanks Marc! I'll try to get a fix in before RDP2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants