TCK tests for async invokers#706
Conversation
|
Draft because the TCK audit already contains changes from jakartaee/cdi#961. Otherwise this is ready for review and then merge. |
|
Rebased. Since jakartaee/cdi#961 was merged, this is ready for review. |
manovotn
left a comment
manovotn
left a comment
There was a problem hiding this comment.
We should also add a test for AfterDeploymentValidation.ensureAsyncHandlerExists()
If anything, we should add a copy of this PR for Portable Extensions. I don't see the need to add a test for this one method, without testing the rest of the infrastructure through PEs. |
That would be even better for sure :) |
|
@Ladicek do you want to address the PE counterpart in this PR should we open a new one? |
|
To be honest, I'd love someone else to do the PE part 😆 |
I have added a PE variant of the three positive test cases - is that OK with you @Ladicek? |
|
Just about perfect, thanks! |
|
Added a 2nd commit that contains TCK tests for the respecified async handlers (jakartaee/cdi#967). Before merging, the commits should be squashed. Asked for a new review. |
|
@manovotn I think I screwed up by force-pushing the new TCK, because that dropped your commit with PEs tests. Do you by any chance still have them locally? Thanks! :-) |
|
Squashed and rebased. @manovotn could you please take a look if you still have your PE tests locally? Sorry I dropped them. |
Pushed |
manovotn
left a comment
manovotn
left a comment
There was a problem hiding this comment.
I went over the tests as I was implementing it in Weld and it LGTM 👍
3 participants
Fixes #698