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

[WFLY-11915] Test case for passing a remote EJB Exception by reference (WFLY-11866) #12191

Closed
wants to merge 1 commit into from

Conversation

@tmiyargi
Copy link
Contributor

tmiyargi commented Mar 28, 2019

@tmiyargi tmiyargi mentioned this pull request Mar 28, 2019
0 of 11 tasks complete
@wfink

This comment has been minimized.

Copy link
Contributor

wfink commented Mar 28, 2019

I would consider the test as not complete. There should be the following tests

  • Pass a parameter which is not serializable
  • Pass a TransferParameter (might be serializable) and change it inside HelloBean and check it after returning
  • return an object which is not marked as Serializable
  • return a serializable object which contain a nor serializable object
  • throw the RemoteByReferenceException
Copy link
Contributor

darranl left a comment

Can we please try and use meaningful commit messages that help in the future.

When looking through the git history in 5 years time or an annotated view of source messages like "Test for WFLY-11866" don't reveal much unless you manually open the referenced Jira issues - when looking at a class with a lot of updates you can end up cross referencing quite a few issues.

@tmiyargi tmiyargi changed the title [WFLY-11866] Test for WFLY-11866 [WFLY-11915] Test case for passing a remote EJB Exception by reference (WFLY-11866) Mar 28, 2019
@tmiyargi tmiyargi force-pushed the tmiyargi:WFLY-11866 branch 2 times, most recently from 3cdb9f6 to 653c528 Mar 28, 2019
@tmiyargi tmiyargi force-pushed the tmiyargi:WFLY-11866 branch from 653c528 to d4a93e0 Apr 4, 2019
@bstansberry

This comment has been minimized.

Copy link
Contributor

bstansberry commented Apr 4, 2019

@tmiyargi In the future please send the test to the original fix author for inclusion in that PR so the fix can be tested automatically by CI.

@wfink is going to cherry-pick this commit into his branch.

@wfink

This comment has been minimized.

Copy link
Contributor

wfink commented Apr 4, 2019

@bstansberry

This comment has been minimized.

Copy link
Contributor

bstansberry commented Apr 4, 2019

Closing as the commit was merged as part of #12167

@bstansberry bstansberry closed this Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.