eclipse / microprofile-context-propagation

Apache License 2.0
33 stars 23 forks source link

Rewrite TC/ME JTA test to use APIs while still allowing for m… #179

Closed manovotn closed 4 years ago

manovotn commented 4 years ago

…issing impls.

Fixes #178

Rewrites test to eliminate reflections usage and instead uses standard APIs.

manovotn commented 4 years ago

I only found this occurrence in one test within ThreadContextTest and once in ManagedExecutorTest, correct me if I missed anything...

manovotn commented 4 years ago

@njr-11 fixes all remarks, thanks for review.

This isn't necessarily a problem because, as I mentioned earlier, our official run of the TCK is with CDI enabled and that still runs cleanly. I'm more pointing it out because it demonstrates what will happen to an implementation that doesn't provide a CDI implementation or spec. If you haven't already, I'd recommend getting a review from @FroMage as well to confirm this won't cause any problems for RestEasy.

I think it also depends on what exactly is a "CDI disabled" mode? The TCK itself now depends on CDI API and what I tried to do here is that so long as the implementation will have the API available (not the impl), then the tests should be runnable. What you described seems to only be possible in multi CL environment where you deliberately cut out some pieces. Or am I missing something else?

And I did mark Stephane for review already, not planning on merging without his review.

manovotn commented 4 years ago

@FroMage whenever you have a minute, please add a review :)

FroMage commented 4 years ago

Sorry about the delay. Merged.