Only fields with name-matching injectables get injected into fullyInitialized SUT #343

Closed
Vampire opened this Issue Sep 22, 2016 · 2 comments

Projects

None yet

2 participants

@Vampire
Vampire commented Sep 22, 2016

According to the JavaDoc of fullyInitialized, each and every field that otherwise stays null should be initialized.

Having the SUT

public class Foo {
    String foo;
    String bar;
}

and the test class

public class FooTest {
    @Tested(fullyInitialized = true)
    Foo sut;

    @Injectable("bam")
    String bam;

    @Injectable("baz")
    String baz;

    @Test
    public void test() {
        System.out.println(sut.foo);
        System.out.println(sut.bar);
    }
}

no field gets filled, though two type-matching injectables are found.

With the test class

public class FooTest {
    @Tested(fullyInitialized = true)
    Foo sut;

    @Injectable("bam")
    String bam;

    @Injectable("bar")
    String bar;

    @Test
    public void test() {
        System.out.println(sut.foo);
        System.out.println(sut.bar);
    }
}

only the bar field is filled.

With the test class

public class FooTest {
    @Tested(fullyInitialized = true)
    Foo sut;

    @Injectable("foo")
    String foo;

    @Injectable("baz")
    String baz;

    @Test
    public void test() {
        System.out.println(sut.foo);
        System.out.println(sut.bar);
    }
}

only the foo field is filled.

With the test class

public class FooTest {
    @Tested(fullyInitialized = true)
    Foo sut;

    @Injectable("foo")
    String foo;

    @Injectable("bar")
    String bar;

    @Test
    public void test() {
        System.out.println(sut.foo);
        System.out.println(sut.bar);
    }
}

both fields are filled.

@rliesenfeld rliesenfeld added the other label Sep 22, 2016
@rliesenfeld
Member
rliesenfeld commented Sep 22, 2016 edited

The behavior in all three tests above is consistent with the API documentation, as far as I can tell (just checked it).

I think you're making a mistake in not realizing that "fullyInitialized = true" is meant for injection points (fields and constructor parameters) for which no @Injectables (or other @Tested fields) are provided in the test class. In all tests above, if only "@Tested" is used the exact same behavior is observed.

@Vampire
Vampire commented Sep 22, 2016

Ah, ok, I didn't realize that without fullyInitialized name-matching injectables are also stuffed into name-matching fields that are not annotated with @Inject or similar.

Now that I read the JavaDoc once again I see the behaviour is at least like documented. Annotated fields are just required, but you can inject all fields, if multiple types, the names must match, if only one type, a type match is sufficient. I guess this is to stay deterministic as to what gets injected where.

But I have one improvement request for this. Wouldn't it be enough to have all but one name-matching and if one uninjected field and one type-matching injectable is left, it can be filled with that just as if only one field would have been defined.

And one quirk maybe still, the JavaDoc says For each such target field, the value of a still unused injectable of the same type is assigned, if any is available. Multiple target fields of the same type can be injected from separate injectables, provided each target field has the same name as an available injectable of that type. According to this, if you remove one of the injectables in my first example (resulting in injectable baz and fields foo and bar), you are actually only trying to inject one field, as you only provide one injectable. I suggest changing the wording, so that it makes clear that if there are multiple target fields of the same type, you have to have name-matching injectables.

And about the fullyInitialized, if it is meant for injection points only, then the JavaDoc is misleading. For this field it starts with the sentence Indicates that each and every field of the tested object should be assigned a value. I do not read anywhere that this only refers to DI annotated fields, but the wording is each and every. So it shouldn't be possible to have empty String fields like in my examples here.

@rliesenfeld rliesenfeld self-assigned this Sep 24, 2016
@rliesenfeld rliesenfeld added enhancement and removed other labels Sep 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment