search

weld / weld-testing

Set of test framework extensions (JUnit 4, JUnit 5, Spock) to enhance the testing of CDI components via Weld. Supports Weld 5.
http://weld.cdi-spec.org/
Apache License 2.0
102 stars 30 forks source link

add recursive search for WeldInitiator in nested test classes #133

Closed JHahnHRO closed 2 years ago

JHahnHRO commented 2 years ago

Will resolve #129 by modifying the search for @WeldSetup WeldInitiator fields so that @Nested test instances are searched first, then their enclosing instance, then their enclosing instance etc. until an Initiator is found. This way, nested test classes can re-use their parent's configuration by doing nothing and overwrite their parent's configuration by defining their own @WeldSetup WeldInitiator field.

JHahnHRO commented 2 years ago

Also note that master is now CDI 4/Weld 5 so if you need this for weld-junit 3 (Weld 4) we will need to backport it.

Yes, I want to backport it to 3.x and preferably even 2.x. But I thought I should wait with PRs for additional branches until the PR to master is accepted.

manovotn commented 2 years ago

Yes, I want to backport it to 3.x and preferably even 2.x. But I thought I should wait with PRs for additional branches until the PR to master is accepted.

3.x is fine and I will backport it there. That branch will get a release once Spock PRs are addressed. However, 2.x isn't actively developed and there is no release plan for it so I don't see much point in backporting there.

JHahnHRO commented 2 years ago

However, 2.x isn't actively developed and there is no release plan for it so I don't see much point in backporting there.

There are still quite a few projects in active development that use javax, for example all Quarkus applications (for now at least). Those will need the javax-Version of Weld for testing (in Quarkus' case because there is no similar testing support for Arc). On the other hand, this is not the most important feature ever, so I guess I can live without it.

Vampire commented 2 years ago

Is it expected / intended, that this PR breaks existing tests? Before this PR it was perfectly fine to have @WeldSetup Object foo = WeldInitiator.of(Foo.class) now this would fail as no longer the value type is checked, but now the field type is checked.

manovotn commented 2 years ago

Is it expected / intended, that this PR breaks existing tests? Before this PR it was perfectly fine to have @WeldSetup Object foo = WeldInitiator.of(Foo.class) now this would fail as no longer the value type is checked, but now the field type is checked.

Definitely not intended but it looks like we didn't have tests for that (not that I would expect people to store it in Object although I see no reason to forbid it). Looks like this is the offending bit, right? Previously, we instead checked if the field value is instanceof WeldInitiator and casted to it.

We should probably keep that.

Vampire commented 2 years ago

Exactly

Vampire commented 2 years ago

I incorporated the logic revert in my other PR where I had other Jupiter module improvements too.

manovotn commented 2 years ago

Thanks!