Skip to content
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

Merged
merged 2 commits into from
Jul 23, 2019

Conversation

chengfang
Copy link
Contributor

…re of a SFSB in that module.

@fl4via
Copy link
Contributor

fl4via commented Jun 28, 2019

@rachmatowicz Since this affects, affinity, can you please review it? thanks!

@rachmatowicz
Copy link
Contributor

@fl4via Yes, but I may not get to it until Monday.

@rachmatowicz
Copy link
Contributor

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?

@chengfang
Copy link
Contributor Author

@rachmatowicz I just run "mvn clean install" in tests directory, with java 1.8:

[INFO] testBuild .......................................... SUCCESS [  0.951 s]
[INFO] secondTestEJB3 ..................................... SUCCESS [  2.279 s]
[INFO] secondTestEJB3_EAR ................................. SUCCESS [  0.556 s]
[INFO] testEJB3Client ..................................... SUCCESS [  1.446 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  5.561 s
[INFO] Finished at: 2019-07-01T15:33:54-04:00
[INFO] ------------------------------------------------------------------------
/Users/cfang/dev/customer/EJBCLIENT-333/tests > java -version
java version "1.8.0_181"

Then once it's built successfully, follow tests/README.txt to configure 2 nodes, deploy testEJB3Client war to node1 and secondTestEJB3_EAR to node2, and start 2 nodes in the same machine with port offset.

@chengfang
Copy link
Contributor Author

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.

@rachmatowicz
Copy link
Contributor

rachmatowicz commented Jul 1, 2019

Maven settings problem. Working now. Thanks.
Finally got the reproducer to work to reproduce the error.

@rachmatowicz
Copy link
Contributor

@chengfang @fl4via Comments on PR on the issue.

@rachmatowicz
Copy link
Contributor

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.

…al handling of NoSuchEJBException in removeNode() method.
@rachmatowicz
Copy link
Contributor

@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?

@chengfang
Copy link
Contributor Author

@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.

@xstefank
Copy link
Member

@chengfang is this good to be merged or are you still working on a testcase, please?

@chengfang
Copy link
Contributor Author

@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.

@xstefank
Copy link
Member

@fl4via can this be merged then?

@chengfang chengfang merged commit 984a04a into wildfly:4.0 Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants