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

Address ManagedBeanTypesTest.testGenericHierarchyBeanTypes failure on Java 21 when using tck-web-suite.xml #513

Closed

Conversation

scottmarlow
Copy link
Contributor

@scottmarlow scottmarlow commented Dec 7, 2023

Also exclude ManagedBeanTypesTest.testGenericHierarchyBeanTypes in tck-web-suite.xml for #485

I'm not trying to use tck-web-suite.xml but seem to be doing so as I see a failure on Java 21 that might also need to be addressed elsewhere. This change is to exclude the ManagedBeanTypesTest.testGenericHierarchyBeanTypes test for tck-web-suite.xml as well.

I'm using:

export TCK_VERSION=4.0.12
mvn clean verify -Dincontainer -Dcdi.tck-4-0.version=${TCK_VERSION} -Dmaven.test.failure.ignore=true

With the above, I seem to using tck-web-suite.xml which doesn't have the same excludes as tck-core-suite.xml.

Test results are:

[INFO] Results:
[ERROR] Failures:
[ERROR] ManagedBeanTypesTest>Arquillian.run:138->testGenericHierarchyBeanTypes:80 Set [class java.lang.Object, java.util.List<org.jboss.cdi.tck.tests.definition.bean.types.Vulture<java.lang.Integer>>, interface java.util.Collection<org.jboss.cdi.tck.tests.definition.bean.types.Vulture<java.lang.Integer>>, interface java.util.SequencedCollection<org.jboss.cdi.tck.tests.definition.bean.types.Vulture<java.lang.Integer>>, class org.jboss.cdi.tck.tests.definition.bean.types.Flock, interface java.lang.Iterable<org.jboss.cdi.tck.tests.definition.bean.types.Vulture<java.lang.Integer>>] (6) does not match array [class java.lang.Object, class org.jboss.cdi.tck.tests.definition.bean.types.Flock, java.util.List<org.jboss.cdi.tck.tests.definition.bean.types.Vulture<java.lang.Integer>>, java.util.Collection<org.jboss.cdi.tck.tests.definition.bean.types.Vulture<java.lang.Integer>>, java.lang.Iterable<org.jboss.cdi.tck.tests.definition.bean.types.Vulture<java.lang.Integer>>] (5)
[ERROR] Tests run: 1833, Failures: 1, Errors: 0, Skipped: 0
[INFO]
[ERROR] There are test failures.

I'm not really sure if this pull request should be merged versus finding out why -Dincontainer is using tck-web-suite.xml which I think would be used when passing -Dincontainer=webprofile. Thoughts?

…t fail if/when tck-web-suite.xml is used instead of tck-core-suite.xml when running "mvn clean verify -Dincontainer -Dcdi.tck-4-0.version= -Dmaven.test.failure.ignore=true" which for me seems to use tck-web-suite.xml instead of tck-core-suite.xml

Signed-off-by: Scott Marlow <[email protected]>
@manovotn
Copy link
Contributor

manovotn commented Dec 8, 2023

I'm not really sure if this pull request should be merged versus finding out why -Dincontainer is using tck-web-suite.xml which I think would be used when passing -Dincontainer=webprofile. Thoughts?

Probably because of this? https://github.com/weld/core/blob/master/jboss-tck-runner/pom.xml#L364-L376
In Weld you can use -Dcdi.tck.suite.xml.file to change the xml file used.

With the above, I seem to using tck-web-suite.xml which doesn't have the same excludes as tck-core-suite.xml.

Standard TCK suit and webprofile contain different tests and so have different XMLs.
Seems like Weld is attempting to execute all tests together? Which wouldn't matter except for the XML file not being one-size-fits-all. And IIRC, testng didn't allow you to declare any value that's not an actual test effectively preventing you from having a singular file.

...used when passing -Dincontainer=webprofile

I don't think such property value is recognized by Weld?

@Ladicek
Copy link
Contributor

Ladicek commented Dec 8, 2023

It is probably fair to expect exclusions from core TCK (impl/src/main/resources/tck-tests.xml) to also exist in the integration TCK (web/src/main/resources/tck-tests.xml). Or, in other words, the impl exclusion list should always be a subset of the web exclusion list, I guess?

@manovotn
Copy link
Contributor

manovotn commented Dec 8, 2023

It is probably fair to expect exclusions from core TCK (impl/src/main/resources/tck-tests.xml) to also exist in the integration TCK (web/src/main/resources/tck-tests.xml). Or, in other words, the impl exclusion list should always be a subset of the web exclusion list, I guess?

Yea, I was thinking the same 👍
It's easy to forget about during updates tho...

@Ladicek
Copy link
Contributor

Ladicek commented Dec 8, 2023

Maybe we could add a test?

EDIT: IIRC, it should be fairly easy using XMLUnit.

@Ladicek
Copy link
Contributor

Ladicek commented Dec 8, 2023

This is what I had in mind:

package org.jboss.cdi.tck.test;

import org.testng.annotations.Test;
import org.xmlunit.builder.DiffBuilder;
import org.xmlunit.diff.ComparisonResult;
import org.xmlunit.diff.ComparisonType;
import org.xmlunit.diff.Diff;
import org.xmlunit.diff.DifferenceEvaluators;

import java.io.IOException;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;

public class ExclusionListsTest {
    @Test
    public void compareExclusionLists() throws IOException {
        List<URL> xmls = new ArrayList<>();
        ExclusionListsTest.class.getClassLoader().getResources("tck-tests.xml").asIterator().forEachRemaining(xmls::add);
        assertEquals(xmls.size(), 2);
        URL control = xmls.stream().filter(it -> !it.toString().contains("web")).findFirst().orElseThrow();
        URL test = xmls.stream().filter(it -> it.toString().contains("web")).findFirst().orElseThrow();

        Diff diff = DiffBuilder.compare(control)
                .withTest(test)
                .checkForSimilar()
                .ignoreComments()
                .ignoreWhitespace()
                .withDifferenceEvaluator(DifferenceEvaluators.chain(
                        DifferenceEvaluators.Default,
                        (comparison, outcome) -> {
                            if (outcome != ComparisonResult.DIFFERENT) {
                                return outcome;
                            }
                            if (comparison.getType() == ComparisonType.CHILD_NODELIST_LENGTH
                                    && (Integer) comparison.getControlDetails().getValue() < (Integer) comparison.getTestDetails().getValue()) {
                                return ComparisonResult.SIMILAR;
                            }
                            if (comparison.getType() == ComparisonType.CHILD_LOOKUP
                                    && comparison.getControlDetails().getValue() == null
                                    && comparison.getTestDetails().getValue() != null) {
                                return ComparisonResult.SIMILAR;
                            }
                            return outcome;
                        }))
                .build();
        assertFalse(diff.hasDifferences(), diff.fullDescription());
    }
}

@Ladicek
Copy link
Contributor

Ladicek commented Dec 8, 2023

Superseded by #514 / #515.

@Ladicek Ladicek closed this Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants