Fix a TCK regression #195
Fix a TCK regression #195
Conversation
…Wrapper Because that didn't work anymore with CompletionStageWrapper that gained an executor now
…on the ContextManager
|
|
||
| public CompletableFutureWrapper(SmallRyeThreadContext context, CompletableFuture<T> f, Executor executor, boolean minimal) { | ||
| public final static int FLAG_MINIMAL = 1 << 0; | ||
| public final static int FLAG_DEPENDENT = 1 << 1; |
There was a problem hiding this comment.
These are really hard to decipher. Was there any special reason why ordinary conditions wouldn't work?
There was a problem hiding this comment.
I was at the point where I had two boolean parameters to the constructor, and that's where normally we switch to enums, but I figured creating two enums Something.MINIMAL/NORMAL SomethingElse.DEPENDENT/WRAPPER didn't feel a lot better than flags.
If you feel strongly in favour of two enums, or perhaps using an EnumSet in a single enum, I can change it, but two boolean in a row are a good reason to find an alternative.
There was a problem hiding this comment.
I see. I don't really have a strong argument to convince you, it's just that for me the readability of bitwise operations for conditions was always really low. If you feel like it's the easiest way to do it, keep it :-)
That only happens on Quarkus because the current TCK doesn't test with an executor service, and Quarkus has one. On Quarkus we'll still have two tests failing until MP-CP releases the new TCK.
Now I've tested with the updated TCK at eclipse/microprofile-context-propagation#197 and we pass.
Also fixed a few bugs spotted while writing the new TCK bits.