Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Optional elresolver updates #1224

Merged

Conversation

markt-asf
Copy link
Contributor

Fixes Issue
No issue. The EL spec has been updated. One change and some additional tests are required.

Related Issue(s)
None.

Describe the change
Updates the existing OptionalELResolver tests for property resolution.
Adds new OptionalELResolver tests for method invocation.
Cleans up the code for the OptionalELResolver tests.

Additional context
None.

CC @alwin-joseph @anajosep @arjantijms @cesarhernandezgt @dblevins @m0mus @edbratt @gurunrao @jansupol @jgallimore @kazumura @kwsutter @LanceAndersen @bhatpmk @RohitKumarJain @shighbar @gthoman @brideck @OndroMih @dmatej
@starksm64 @scottmarlow

@alwin-joseph
Copy link
Contributor

Will verify when new API jar is released after jakartaee/expression-language#208 is merged.

@markt-asf
Copy link
Contributor Author

Tx. 6.0.0-RC1 has been published that includes that change.

@alwin-joseph
Copy link
Contributor

I encountered 1 test failure , using GlassfishM2 (and copying el api 6.0.0-RC1 jar). Other new tests passed.

Errors:
[ERROR] ELClientIT.optionalELResolverObjectInvoke:227->testOptionalELResolverInvoke:270
[ERROR] Tests run: 361, Failures: 0, Errors: 1, Skipped: 0

java.lang.NullPointerException: Cannot read the array length because "<local7>" is null
        at com.sun.ts.tests.el.api.jakarta_el.optionalelresolver.ELClientIT.testOptionalELResolverInvoke(ELClientIT.java:270)
        at com.sun.ts.tests.el.api.jakarta_el.optionalelresolver.ELClientIT.optionalELResolverObjectInvoke(ELClientIT.java:227)

@markt-asf
Copy link
Contributor Author

A fix will be required in Glassfish (ExpressLy?) for that. The expected behaviour has changed.

@alwin-joseph
Copy link
Contributor

A fix will be required in Glassfish (ExpressLy?) for that. The expected behaviour has changed.

Okey. I will merge this and update the bundle at https://www.eclipse.org/downloads/download.php?file=/ee4j/jakartaee-tck/jakartaee11/staged/eftl/jakarta-expression-language-tck-6.0.0.zip. Will retest when expressly has the required changes integrated.

@alwin-joseph alwin-joseph merged commit 94f463f into jakartaee:tckrefactor Feb 26, 2024
2 of 3 checks passed
@arjantijms
Copy link
Contributor

A fix will be required in Glassfish (ExpressLy?) for that. The expected behaviour has changed.

I'll take a look at that soon (should be in M3 then). Thanks!

@arjantijms
Copy link
Contributor

A fix will be required in Glassfish (ExpressLy?) for that. The expected behaviour has changed.

As it appeared this issue was entirely in the API jar. I do wonder how Tomcat was able to pass this, unless a different API jar was used.

@markt-asf
Copy link
Contributor Author

Tomcat has always provided its own API JARs for all the specs it implements.

@arjantijms
Copy link
Contributor

Tomcat has always provided its own API JARs for all the specs it implements.

Thanks, that clears it up.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants