-
Notifications
You must be signed in to change notification settings - Fork 112
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
Update Excludes for Accepted TCK Challenge: https://github.com/eclipse-ee4j/jaxrs-api/issues/937 #603
Conversation
While we agreed that CDI possibly can create a proxy, the cause seems to be the same as in the case of the second TCK issue, where the discussion still continues. Similar to that second issue, the ExceptionMapper is not a CDI bean, since it does not have any bean defining annotations. In the case of the bean validation, it is not clear if the TCK test classes should be treated as CDI beans, and in this case, it is not clear whether the TCK test ExceptionMapper should have been a CDI bean. I believe the CDI related exclusions should go all or none, based on the conclusion of discussion in the Jakarta REST issue tracker. |
Based on jakartaee/rest#937 (comment) I agree with the exclude. However, this exclusion would not work. For the exclusion to work, the tests need to be specified with the vehicles, if I remember correctly. I.e.
|
Ah, yes. I can make an update to that today with the proper vehicle added. |
Jan,
If you recall, we used to have a tool to verify the exclude list entries including which containers, if any the tests run in. Do we have something similar for the Jakarta TCK projects?
On Jan 27, 2021, at 11:40 AM, jansupol ***@***.***> wrote:
based on Based on jakartaee/rest#937 (comment) <jakartaee/rest#937 (comment)> I agree with the exclude. However, this exclusion would not work. For the exclusion to work, the tests need to be specified with the vehicles, if I remember correctly. I.e.
com/sun/ts/tests/jaxrs/ee/rs/ext/providers/JAXRSProvidersClient.java#isRegisteredRuntimeExceptionMapper_from_standalone
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#603 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQPFO57DU5AI3PK7Q3P3JDS4A6XPANCNFSM4WULZA3A>.
My best,
Lance
--
Lance Andersen
USTA EMA President and CEO
PTR Professional 5A
USPTA Elite Professional
TIA Global Cardio Tennis-Master Trainer
USRSA
Mobile: 978 857-0446
luckydogtennis.com -- luckydogtennis.com/TennisBlog -- cardiotennis.com
|
@LanceAndersen I never used a tool. Perhaps it is one in the tools repo? |
Hi Jan,
Yes I believe that is the one
On Jan 27, 2021, at 1:19 PM, jansupol ***@***.***> wrote:
This one looks promising <https://github.com/eclipse-ee4j/jakartaee-tck-tools/tree/master/tools/exclude-utils>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#603 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQPFO25ONWV6GXODWT66ODS4BKKFANCNFSM4WULZA3A>.
My best,
Lance
--
Lance Andersen
USTA EMA President and CEO
PTR Professional 5A
USPTA Elite Professional
TIA Global Cardio Tennis-Master Trainer
USRSA
Mobile: 978 857-0446
luckydogtennis.com -- luckydogtennis.com/TennisBlog -- cardiotennis.com
|
I'm not actually sure if the vehicle is required, unless the test method in question is run with multiple vehicles and only some of those are intended to be excluded. The recent Servlet exclusions that I was using as a template do not include a vehicle suffix, and they successfully excluded the test. Regardless, the test method was incorrect in my initial commit, so it's good that I took a closer look at this. It should now be correct. |
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.
This looks much better. The signoff is missing though,
We should start requesting the .jtr test result files to be attached for confirming the tests to be excluded (should be the line that starts with |
This comment has been minimized.
This comment has been minimized.
4cfcb5e
to
f2b1988
Compare
jakartaee/rest#937 Signed-off-by: Brian Decker <[email protected]>
f2b1988
to
f03c201
Compare
Okay, I've successfully squashed my commits and amended in the signoff so I think it's good now. I knew I was missing it, but didn't know what the process was. Long time listener, first time caller. |
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.
LGTM
I downloaded the .jtr from jakartaee/rest#937 and confirm that it contains the following which matches the Jakarta EE Platform TCK change in this pr: Note that the |
name: Pull Request
about: Create a pull request for a Platform TCK change
title: ''
labels: ''
assignees: ''
Fixes Issue
Specify the issue (link) that is solved with this pull request.
Related Issue(s)
Specify any related issue(s) links.
Describe the change
A clear and concise description of the change.
Additional context
Add any other context about the problem here.
CC @alwin-joseph @anajosep @arjantijms @cesarhernandezgt @dblevins @m0mus @edbratt @gurunrao @jansupol @jgallimore @kazumura @kwsutter @LanceAndersen @bhatpmk @RohitKumarJain @gthoman