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

Jackson212MissingConstructorTest still fails with 2.15.0-SNAPSHOT #14

Closed
henrik242 opened this issue Jan 18, 2023 · 3 comments
Closed

Comments

@henrik242
Copy link
Contributor

henrik242 commented Jan 18, 2023

Jackson212MissingConstructorTest should have been fixed now according to FasterXML/jackson-dataformat-xml#547 and FasterXML/jackson-module-kotlin#396 (comment)

But it still fails:

$ mvn clean test -Dtest=Jackson212MissingConstructorTest
[...]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running com.fasterxml.jackson.failing.Jackson212MissingConstructorTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.631 s <<< FAILURE! - in com.fasterxml.jackson.failing.Jackson212MissingConstructorTest
[ERROR] testMissingConstructor(com.fasterxml.jackson.failing.Jackson212MissingConstructorTest)  Time elapsed: 0.591 s  <<< FAILURE!
junit.framework.AssertionFailedError: expected:<Product(stuff=null)> but was:<Product(stuff=Stuff(str=null))>
	at com.fasterxml.jackson.failing.Jackson212MissingConstructorTest.testMissingConstructor(Jackson212MissingConstructorTest.kt:24)

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   Jackson212MissingConstructorTest.testMissingConstructor:24 expected:<Product(stuff=null)> but was:<Product(stuff=Stuff(str=null))>
[INFO] 
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
[INFO] 

Tested with jackson-integration-tests commit ddeb914

@cowtowncoder
Copy link
Member

Ok, I am able to see this with Idea; test works as expected. So, as per XML#547, construction no longer fails.
The issue, if any, is what result is expected. There are 2 parts to this:

  1. What is the specific output wanted, and
  2. Which annotations (if any) are needed

Looking at XML module's test, specifically, there are 2 variants to consider:

  • Inner class uses DELEGATING constructor (with nominal argument of String) -- should get empty String ("")
  • Inner class uses PROPERTIES constructor -- should be passed null (no child properties found)

There are separate tests for both cases, but with annotations to indicate which creator to use.

I think that things work as they should, but test expects "wrong" outcome: that could only be result of using Delegating creator -- and in this case that'd need to be annotated.

WDYT @henrik242 ? Adding @JsonCreator(mode = JsonCreator.Mode.DELEGATING) would give result test expects.

    static class Outer547Del {
        public Inner547Del inner;
    }

    static class Inner547Del {
        protected String value;

        @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
        public Inner547Del(@JsonProperty("value") String v) {
            value = v;
        }
    }

    static class Outer547Props {
        public Inner547Props inner;
    }

    static class Inner547Props {
        protected String value;

        // 20-Nov-2022, tatu: [dataformat-xml#547] Shouldn't need annotation
//        @JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
        public Inner547Props(@JsonProperty("value") String v) {
            value = v;
        }
    }

    // [dataformat-xml#547]
    public void testNested1ArgCtorsDelegating() throws Exception
    {
        String xml = "<outer><inner></inner></outer>";
        Outer547Del result = XML_MAPPER.readValue(xml, Outer547Del.class);
        assertNotNull(result.inner);
        assertEquals("", result.inner.value);
    }

    // [dataformat-xml#547]
    public void testNested1ArgCtorsProps() throws Exception
    {
        String xml = "<outer><inner></inner></outer>";
        Outer547Props result = XML_MAPPER.readValue(xml, Outer547Props.class);
        assertNotNull(result.inner);
        assertNull(result.inner.value);
    }

@henrik242
Copy link
Contributor Author

@cowtowncoder As mentioned in the code comment, I think your changes to the test are valid :)

@cowtowncoder
Copy link
Member

@henrik242 Ok! Will close this, leave Kotlin issue as resolved (although would be good to know which versions do have issues in-between)

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

No branches or pull requests

2 participants