jstl-api
jstl-api copied to clipboard
Tags 3.0 TCK Challenge for test ExceptionCheckTag.java depending on removed Pages 4.0 JspException.getRootCause() method...
JspException.getRootCause() was removed from Pages 4.0 and if a Tags 3.0 implementation fails certain Tags 3.0 TCK tests, the cause of failure is unknown due to https://github.com/jakartaee/platform-tck/blob/10.0.x/src/com/sun/ts/tests/jstl/common/tags/ExceptionCheckTag.java#L174 calling the removed JspException.getRootCause() which results in the TCK failure cause not being shown.
The combination of Tags 3.0 + Pages 4.0 Specifications are expected to be used together in Jakarta EE 11 implementations.
Tags 3.0 TCK cannot depend on Pages SPEC API methods removed in Pages 4.0. Since the Tags 3.0 TCK can instead use JspException.getCause(), the Tags 3.0 TCK should be updated to use JspException.getCause() instead of JspException.getRootCause().
Details on the failure from https://github.com/jakartaee/tags/issues/255#issuecomment-1994755183:
[javatest.batch] 01-30-2024 13:00:54: TRACE: [WIRE] - << "
Eclipse GlassFish 8.0.0 - Error report HTTP Status 500 - Internal Server Errortype Exception report
messageInternal Server Error
descriptionThe server encountered an internal error that prevented it from fulfilling this request.
exception
java.lang.NoSuchMethodError: 'java.lang.Throwable jakarta.servlet.jsp.JspException.getRootCause()'
@scottmarlow just to set expectations, is this issue something you'd like one of the Jakarta Tags committers to complete in the platform-tck? Which branch? 10.0?
@scottmarlow just to set expectations, is this issue something you'd like one of the Jakarta Tags committers to complete in the platform-tck? Which branch? 10.0?
Updated: I expect that one of the Platform TCK committers will create a pull request to update the tests to use getCause instead of getRootCause() and stage a Platform TCK for testing (as well as a Tags TCK).
+1 if anyone else wants to create the pull request against https://github.com/jakartaee/platform-tck/tree/10.0.x (need to update ts.jtx files + https://github.com/jakartaee/platform-tck/blob/10.0.x/release/tools/jakartaee.xml + https://github.com/jakartaee/platform-tck/blob/10.0.x/release/tools/jstl.xml)
@scottmarlow we shouldn't exclude the tests but rather use getCause() rather than getRootCause()
@scottmarlow we shouldn't exclude the tests but rather use
getCause()rather thangetRootCause()
Good point and agreed!
Updated: Note that I updated my previous comment today on March 15, 2024.
@arjantijms @volosied @m0mus @jgallimore Given the information here any objection to accepting this challenge? I'm tagging several committers in the project to work to a consensus.
Maling list thread: https://www.eclipse.org/lists/jstl-dev/msg00134.html
@scottmarlow, I've accepted this challenge. Let me know if any further action is required from the Jakarta Tags team.
We will merge the https://github.com/jakartaee/platform-tck/pull/1219 change as soon as possible that will exclude the failing tests.
We will merge the jakartaee/platform-tck#1219 change as soon as possible that will exclude the failing tests.
@scottmarlow I thought we had agreed that these tests would be updated and not excluded: https://github.com/jakartaee/tags/issues/256#issuecomment-1997660685
We will merge the jakartaee/platform-tck#1219 change as soon as possible that will exclude the failing tests.
@scottmarlow I thought we had agreed that these tests would be updated and not excluded: #256 (comment)
Thanks @pnicolucci I really appreciate your catching me overlooking that!
@scottmarlow were these tests ever updated or do you need help with a PR? Do we need to remove the exclusions: https://github.com/jakartaee/platform-tck/pull/1219.
@scottmarlow were these tests ever updated or do you need help with a PR? Do we need to remove the exclusions: https://github.com/jakartaee/platform-tck/pull/1219.
@pnicolucci https://github.com/jakartaee/platform-tck/pull/1219 did not exclude the Tags tests as that part of the pull request was reverted.
Thanks for volunteering! Yes, please update the the Tags tests to use JspException.getCause() instead of JspException.getRootCause().
@scottmarlow I should have a set of PRs up for this tonight sometime, I'm just validating all the tests pass.
PRs are up and ready for review!
@scottmarlow can we close this issue out?
Thank you for your help @pnicolucci!