-
Notifications
You must be signed in to change notification settings - Fork 914
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARTEMIS-856 Test fixes #2209
ARTEMIS-856 Test fixes #2209
Conversation
@michaelandrepearce I need you to take a look into this please. MessageRedistributionFix is broken, up to the point I can't complete a whole testsuite on my internal CI. My idea was the emulate the previous collection where redistributor would be an item, while still checking if redistributor is the only allowed item to be used on the loop. I'm not sure how else we would change this without making substantial refactoring to your previous commit. Can you look into this PR please? |
@michaelandrepearce : But please don't merge it yet.. I'm running a whole testsuite on the CI before we can merge this... just let me know what you think first. |
ab25403
to
0b4c6e4
Compare
<<<< I had written here, asking Michael to take a look on ExclusiveTest, but I later figured it out.. so I removed the comment here .. :) >>>> |
26b5d06
to
e75965f
Compare
@clebertsuconic, looks good, theres one bit of debug code youve left in that needs to be removed., there we some bits with consumer lists with the redistributor i heavily want to avoid, im working an alternative option, but if to fix master happy to merge this once the debug code is removed, and i can supply my alternative later (have raised a pr so you can see, but still not complete) |
@@ -544,6 +560,7 @@ public boolean isExclusive() { | |||
|
|||
@Override | |||
public synchronized void setExclusive(boolean exclusive) { | |||
new Exception("exclusive set at " + exclusive).printStackTrace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this needs to be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. That’s debug I forgot :p
This is replaced by #2211 |
No description provided.