-
Notifications
You must be signed in to change notification settings - Fork 133
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
EJBCLIENT-333 Unable to invoke any EJB of the same module after failu… #399
Conversation
…re of a SFSB in that module.
@rachmatowicz Since this affects, affinity, can you please review it? thanks! |
@fl4via Yes, but I may not get to it until Monday. |
I'm looking at this example now, the reproducer doesn't build correctly for me, so spending some time with that. Cheng, any tips on what you did to get it running? |
@rachmatowicz I just run "mvn clean install" in tests directory, with java 1.8:
Then once it's built successfully, follow |
I found a bit hard to understand the tests at first, so I put a note in the jira comment explaining it a little more. The reproducer is probably adapted from some other case. Other than that, I was able to reproduce it consistently, and verify the fix with the reproducer. |
Maven settings problem. Working now. Thanks. |
@chengfang @fl4via Comments on PR on the issue. |
Might be worth adding a comment referring to this issue at the point where you introduce the check for stateful bean, to explain why the condition is needed. |
src/main/java/org/jboss/ejb/protocol/remote/RemotingEJBClientInterceptor.java
Show resolved
Hide resolved
…al handling of NoSuchEJBException in removeNode() method.
@chengfang @fl4via Sorry, one other point which comes to mind. Should we be in the habit of writing a test case to validate every bug fixed and every new feature added, in order to expand the test coverage of the component test suites? |
@rachmatowicz I agree we should strive to write tests along with bug fixes and new features, especially unit tests. For more complex use case testing and integration tests (such as this issue), which usually requires complex setup and testing framework, it's best handled by dedicated QE tests. |
@chengfang is this good to be merged or are you still working on a testcase, please? |
@xstefank No, I'm not working on tests for this issue. As I commented earlier, this is one of those issues that are not easily testable by dev tests. So I think this PR is ready for merge. |
@fl4via can this be merged then? |
…re of a SFSB in that module.