jstl-api icon indicating copy to clipboard operation
jstl-api copied to clipboard

Tags 3.0 TCK Challenge against tests that include space character before AM/PM

Open scottmarlow opened this issue 1 year ago • 37 comments
trafficstars

The Java rules have changed as per https://bugs.openjdk.org/browse/JDK-8304925 (Some date/time strings created with JDK <=19 can not be parsed since JDK 20) + https://bugs.openjdk.org/browse/JDK-8284840 (Update CLDR to Version 42.0) which means that the Tags 3.0 TCK tests are now seeing failures like Caused by: java.text.ParseException: Unparseable date: "Nov 21, 2000, 3:45:02 AM".

https://github.com/jakartaee/platform-tck/blob/10.0.x/src/web/jstl/compat/onedotzero/positiveFormatLocalizationContextI18NTest.jsp#L32 contains <c:set var="dt" value="Nov 21, 2000, 3:45:02 AM"/> which I do not think can be updated to work on Java 21 (e.g. to use non-breaking space) as that would mean the test would fail on Java 11/17 (which needs a breaking space).

Are there other possible fixes that we should consider making?

Note that the https://github.com/jakartaee/platform-tck/blob/10.0.x/src/web/jstl files that contain PM are:

  • jstl/spec/fmt/format/fmtdate/positiveFDTimeZonePrecedenceTest.jsp
  • jstl/compat/onedotzero/positiveSetTimezoneValueTest.gf
  • jstl/compat/onedotzero/positiveTimezoneValueTest.gf
  • jstl/spec/fmt/format/fmtdate/positiveFDDateStyleTest.gf
  • jstl/spec/fmt/format/fmtdate/positiveFDFallbackLocaleTest.gf
  • jstl/spec/fmt/format/fmtdate/positiveFDLocalizationContextTest.gf
  • jstl/spec/fmt/format/fmtdate/positiveFDTimeStyleTest.gf
  • jstl/spec/fmt/format/fmtdate/positiveFDTimeZoneNullEmptyTest.gf
  • jstl/spec/fmt/format/fmtdate/positiveFDTimeZonePrecedenceTest.gf
  • jstl/spec/fmt/format/fmtdate/positiveFDTimeZoneTest.gf
  • jstl/spec/fmt/format/fmtdate/positiveFDTypeTest.gf
  • jstl/spec/fmt/format/settimezone/positiveSetTimezoneValueTest.gf
  • jstl/spec/fmt/format/timezone/positiveTimezoneValueTest.gf

Note that the https://github.com/jakartaee/platform-tck/blob/10.0.x/src/web/jstl files that contain AM are:

  • jstl/compat/onedotzero/positiveFormatLocalizationContextI18NTest.gf
  • jstl/compat/onedotzero/positiveFormatLocalizationContextI18NTest.jsp
  • jstl/compat/onedotzero/positiveSetTimezoneValueTest.gf
  • jstl/compat/onedotzero/positiveTimezoneValueTest.gf
  • jstl/spec/fmt/format/localecontext/positiveFormatLocalizationContextBrowserLocaleTest.gf
  • jstl/spec/fmt/format/localecontext/positiveFormatLocalizationContextBrowserLocaleTest.jsp
  • jstl/spec/fmt/format/localecontext/positiveFormatLocalizationContextBundleTest.gf
  • jstl/spec/fmt/format/localecontext/positiveFormatLocalizationContextBundleTest.jsp
  • jstl/spec/fmt/format/localecontext/positiveFormatLocalizationContextI18NTest.gf
  • jstl/spec/fmt/format/localecontext/positiveFormatLocalizationContextI18NTest.jsp
  • jstl/spec/fmt/format/localecontext/positiveFormatLocalizationContextLocaleTest.gf
  • jstl/spec/fmt/format/localecontext/positiveFormatLocalizationContextLocaleTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDBodyValueTest.gf
  • jstl/spec/fmt/format/parsedate/positivePDBodyValueTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDDateStyleTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDFallbackLocaleTest.gf
  • jstl/spec/fmt/format/parsedate/positivePDFallbackLocaleTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDLocalizationContextTest.gf
  • jstl/spec/fmt/format/parsedate/positivePDLocalizationContextTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDParseLocaleTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDScopeTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDTimeStyleTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDTimeZoneNullEmptyTest.gf
  • jstl/spec/fmt/format/parsedate/positivePDTimeZoneNullEmptyTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDTimeZonePrecedenceTest.gf
  • jstl/spec/fmt/format/parsedate/positivePDTimeZonePrecedenceTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDTimeZoneTest.gf
  • jstl/spec/fmt/format/parsedate/positivePDTimeZoneTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDTypeTest.gf
  • jstl/spec/fmt/format/parsedate/positivePDTypeTest.jsp
  • jstl/spec/fmt/format/parsedate/positivePDVarTest.jsp
  • jstl/spec/fmt/format/settimezone/positiveSetTimezoneValueNullEmptyTest.gf
  • jstl/spec/fmt/format/settimezone/positiveSetTimezoneValueTest.gf
  • jstl/spec/fmt/format/timezone/positiveTimezoneValueNullEmptyTest.gf
  • jstl/spec/fmt/format/timezone/positiveTimezoneValueTest.gf

The good news is that if we do a service release for jakarta-tags-tck-3.0.1.zip that excludes the tests that include ^ breaking space before AM/PM, we will use the same jakarta-tags-tck-3.0.1.zip for validating Standalone Tags 3.0 implementations for Jakarta EE 10 + 11.

We also need to update the EE 11 Platform TCK (tckrefactor branch) to be able to pass (or exclude) the same tests on Java 17 + 21+.

Please see comment https://github.com/jakartaee/platform-tck/pull/1219#issuecomment-1984774938 for more information on the failure.

This TCK challenge is for the following 29 tests that fail on Java 21 with WildFly:

  1. com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveFormatLocalizationContextI18NTest
  2. com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveSetTimezoneValueTest
  3. com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveTimezoneValueTest
  4. com/sun/ts/tests/jstl/spec/fmt/format/fmtdate/JSTLClient.java#positiveFDDateStyleTest
  5. com/sun/ts/tests/jstl/spec/fmt/format/fmtdate/JSTLClient.java#positiveFDFallbackLocaleTest
  6. com/sun/ts/tests/jstl/spec/fmt/format/fmtdate/JSTLClient.java#positiveFDLocalizationContextTest
  7. com/sun/ts/tests/jstl/spec/fmt/format/fmtdate/JSTLClient.java#positiveFDTimeStyleTest
  8. com/sun/ts/tests/jstl/spec/fmt/format/fmtdate/JSTLClient.java#positiveFDTimeZoneNullEmptyTest
  9. com/sun/ts/tests/jstl/spec/fmt/format/fmtdate/JSTLClient.java#positiveFDTimeZonePrecedenceTest
  10. com/sun/ts/tests/jstl/spec/fmt/format/fmtdate/JSTLClient.java#positiveFDTimeZoneTest
  11. com/sun/ts/tests/jstl/spec/fmt/format/fmtdate/JSTLClient.java#positiveFDTypeTest
  12. com/sun/ts/tests/jstl/spec/fmt/format/localecontext/JSTLClient.java#positiveFormatLocalizationContextBrowserLocaleTest
  13. com/sun/ts/tests/jstl/spec/fmt/format/localecontext/JSTLClient.java#positiveFormatLocalizationContextBundleTest
  14. com/sun/ts/tests/jstl/spec/fmt/format/localecontext/JSTLClient.java#positiveFormatLocalizationContextI18NTest
  15. com/sun/ts/tests/jstl/spec/fmt/format/localecontext/JSTLClient.java#positiveFormatLocalizationContextLocaleTest
  16. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDBodyValueTest
  17. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDDateStyleTest
  18. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDFallbackLocaleTest
  19. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDLocalizationContextTest
  20. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDParseLocaleTest
  21. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDTimeStyleTest
  22. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDTimeZoneNullEmptyTest
  23. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDTimeZonePrecedenceTest
  24. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDTimeZoneTest
  25. com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#positivePDTypeTest
  26. com/sun/ts/tests/jstl/spec/fmt/format/settimezone/JSTLClient.java#positiveSetTimezoneValueNullEmptyTest
  27. com/sun/ts/tests/jstl/spec/fmt/format/settimezone/JSTLClient.java#positiveSetTimezoneValueTest
  28. com/sun/ts/tests/jstl/spec/fmt/format/timezone/JSTLClient.java#positiveTimezoneValueNullEmptyTest
  29. com/sun/ts/tests/jstl/spec/fmt/format/timezone/JSTLClient.java#positiveTimezoneValueTest

We also need to challenge the same ^ 29 tests for GlassFish.

scottmarlow avatar Mar 12 '24 14:03 scottmarlow

@scottmarlow why the additional set of test failures in Glassfish? Did those tests pass successfully on Wildfly?

pnicolucci avatar Mar 13 '24 00:03 pnicolucci

[javatest.batch] 01-30-2024 13:10:45:  TRACE: [WIRE] - << "HTTP/1.1 500 Internal Server Error[\r][\n]"
[javatest.batch] 01-30-2024 13:10:45:  TRACE: [WIRE] - << "HTTP/1.1 500 Internal Server Error[\r][\n]"

Is it possible to get the server.log file?

dmatej avatar Mar 13 '24 09:03 dmatej

Is it possible to get the server.log file?

The server.log is not currently archived. The generated JSTL tck contains a copy of the https://github.com/jakartaee/platform-tck/blob/master/docker/jstltck.sh script. I'm not sure if we would need to modification of that or the Jenkins job running the TCK.

@scottmarlow why the additional set of test failures in Glassfish? Did those tests pass successfully on Wildfly?

When GlassFish is running against the JSTL standalone TCK, we see the following test stats: Test results: passed: 502; failed: 40 Tests run == 542

With WildFly running the JSTL tests in EE 10 Platform TCK, we see the following test stats: Test results: passed: 512; failed: 29 Tests run == 541

The Standalone JSTL TCK test count includes one more test as that is the signature test: com/sun/ts/tests/signaturetest/jstl/JSTLClient.java#signatureTest

scottmarlow avatar Mar 13 '24 12:03 scottmarlow

We also need to challenge the same ^ 29 tests for GlassFish and in addition, the following 11 tests as well which also failed as shown in jakartaee-tck-build-run/job/jdk21-run/39/artifact/jstltck.log:

com/sun/ts/tests/jstl/spec/core/urlresource/importtag/JSTLClient.java#negativeImportRequestDispatcherServletExceptionTest
com/sun/ts/tests/jstl/spec/core/urlresource/importtag/JSTLClient.java#negativeImportRequestDispatcherRTIOExceptionTest
com/sun/ts/tests/jstl/spec/fmt/format/fmtnum/JSTLClient.java#negativeFNUnableToParseValueTest
com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#negativePDUnableToParseValueTest
com/sun/ts/tests/jstl/spec/fmt/format/parsenum/JSTLClient.java#negativePNUnableToParseValueTest
com/sun/ts/tests/jstl/spec/sql/param/JSTLClient.java#negativeParamQueryBodyContentTest
com/sun/ts/tests/jstl/spec/sql/param/JSTLClient.java#negativeParamQuerySQLAttributeTest
com/sun/ts/tests/jstl/spec/sql/query/JSTLClient.java#negativeQueryBodyContentTest
com/sun/ts/tests/jstl/spec/sql/query/JSTLClient.java#negativeQuerySQLAttributeTest
com/sun/ts/tests/jstl/spec/sql/update/JSTLClient.java#negativeUpdateBodyContentTest
com/sun/ts/tests/jstl/spec/sql/update/JSTLClient.java#negativeUpdateSQLAttributeTest2

Hey @scottmarlow , my question was more focused on why are the above 11 tests failing in GlassFish but not WildFly. Looking at the output:

[javatest.batch] 01-30-2024 13:00:54: TRACE: [WIRE] - << "

Eclipse GlassFish 8.0.0 - Error report

HTTP Status 500 - Internal Server Error


type 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()'

note The full stack traces of the exception and its root causes are available in the Eclipse GlassFish 8.0.0 logs.


Eclipse GlassFish 8.0.0

The JspException.getRootCause() method was @deprecated As of JSP 2.1, replaced by {@link #getCause()}: https://github.com/jakartaee/pages/blob/master/spec/src/main/asciidoc/ServerPages.adoc#c1-changes-between-jsp-40-and-jsp-31 / https://github.com/jakartaee/pages/pull/234. As can be seen in the Pages 4.0 Changelog and the issue, the getRootCause() method was removed in Pages 4.0 (Jakarta EE11)

Since these 11 tests passed on WildFly but failed on GlassFish, I believe these failures are potentially not Jakarta Tags / TCK issues. If we can see the stack of the exception would give additional context but I didn't see JspException.getRootCause() being referenced anyplace in the tck here: https://github.com/jakartaee/platform-tck/tree/10.0.x/src/web/jstl or in the Jakarta Tags code base here: https://github.com/jakartaee/tags/tree/3.x

To me, it looks like the two sets of tests referenced here are two different problems.

pnicolucci avatar Mar 13 '24 15:03 pnicolucci

Interesting find, the source is in a different folder https://github.com/jakartaee/platform-tck/blob/10.0.x/src/com/sun/ts/tests/jstl/common/tags/ExceptionCheckTag.java#L174

scottmarlow avatar Mar 13 '24 18:03 scottmarlow

@scottmarlow Given this information these 11 tests would only fail where Pages 4.0 / Jakarta EE11 were being tested. Do we need another challenge for this issue or could the TCK just be cleaned up to not call methods removed in Jakarta EE11? It would be good to keep these problems separate IMO.

pnicolucci avatar Mar 13 '24 18:03 pnicolucci

@scottmarlow Given this information these 11 tests would only fail where Pages 4.0 / Jakarta EE11 were being tested. Do we need another challenge for this issue or could the TCK just be cleaned up to not call methods removed in Jakarta EE11? It would be good to keep these problems separate IMO.

Good question, I don't think these 11 tests need a TCK challenge created for them. I think instead we could clean up https://github.com/jakartaee/platform-tck/blob/10.0.x/src/com/sun/ts/tests/jstl/common/tags/ExceptionCheckTag.java#L174 to call JspException.getCause() which I agree should be captured by a separate Tags issues.

scottmarlow avatar Mar 13 '24 19:03 scottmarlow

I'm going to create the Tags issue for reporting the problem with https://github.com/jakartaee/platform-tck/blob/10.0.x/src/com/sun/ts/tests/jstl/common/tags/ExceptionCheckTag.java#L174

I'll also remove the following 11 tests for GlassFish that I had included in this TCK challenge description:

ll which also failed as shown in jakartaee-tck-build-run/job/jdk21-run/39/artifact/jstltck.log:

com/sun/ts/tests/jstl/spec/core/urlresource/importtag/JSTLClient.java#negativeImportRequestDispatcherServletExceptionTest
com/sun/ts/tests/jstl/spec/core/urlresource/importtag/JSTLClient.java#negativeImportRequestDispatcherRTIOExceptionTest
com/sun/ts/tests/jstl/spec/fmt/format/fmtnum/JSTLClient.java#negativeFNUnableToParseValueTest
com/sun/ts/tests/jstl/spec/fmt/format/parsedate/JSTLClient.java#negativePDUnableToParseValueTest
com/sun/ts/tests/jstl/spec/fmt/format/parsenum/JSTLClient.java#negativePNUnableToParseValueTest
com/sun/ts/tests/jstl/spec/sql/param/JSTLClient.java#negativeParamQueryBodyContentTest
com/sun/ts/tests/jstl/spec/sql/param/JSTLClient.java#negativeParamQuerySQLAttributeTest
com/sun/ts/tests/jstl/spec/sql/query/JSTLClient.java#negativeQueryBodyContentTest
com/sun/ts/tests/jstl/spec/sql/query/JSTLClient.java#negativeQuerySQLAttributeTest
com/sun/ts/tests/jstl/spec/sql/update/JSTLClient.java#negativeUpdateBodyContentTest
com/sun/ts/tests/jstl/spec/sql/update/JSTLClient.java#negativeUpdateSQLAttributeTest2

scottmarlow avatar Mar 13 '24 19:03 scottmarlow

@scottmarlow why is the remaining issue with the space character before AM/PM a challenge but #256 is not? These both seem to stem from running the Tags 3.0 TCK on a Java version / Features for Jakarta EE11. Could the TCK be updated for these tests to check the java version and then act accordingly?

pnicolucci avatar Mar 14 '24 23:03 pnicolucci

why is the remaining issue with the space character before AM/PM a challenge but https://github.com/jakartaee/tags/issues/256 is not?

Great question to raise, thanks!

These both seem to stem from running the Tags 3.0 TCK on a Java version / Features for Jakarta EE11.

issues/256 is caused by Java SE 21 breaking backward compatibility via the JDK-8304925 change. #256 is caused by the planned removal of JspException.getRootCause() which the Tags 3.0 TCK depends on. In one case, the TCK failure cause is a breaking change in an EE Specification and in the other, the TCK Failure cause is a breaking change in Java SE 21.

So which of these issues should be TCK challenges? Pasting from the TCK Process guidance on Valid Challenges:

Any test case (e.g., test class, @Test method), test case configuration (e.g., deployment descriptor), test beans, annotations, and other resources considered part of the TCK may be challenged.

The following scenarios are considered in scope for test challenges:

  • Claims that a test assertion conflicts with the specification.

  • Claims that a test asserts requirements over and above that of the specification.

  • Claims that an assertion of the specification is not sufficiently implementable.

  • Claims that a test is not portable or depends on a particular implementation.

It seems to me that the last point (Claims that a test is not portable or depends on a particular implementation.) could be applied to make both issues TCK challenges so I updated #256 title to be a TCK challenge (not that the title is that important but please do add the TCK label to that issue).

Could the TCK be updated for these tests to check the java version and then act accordingly?

Yes, Flavia suggested something like that on Zulip a few days ago. I'm not really sure though about the idea of converting the string based on the Java version (quoting that suggestion from the WildFly Zulip discussion) but I don't really know enough what is the best approach. So what would be the best way to act accordingly? Have different copies of certain JSP files that work on Java 21 and conditionally use them when running on the Java 21? Or some way to convert the date values in the JSP source before the JSP files are processed.

To help answer ^, I'm pasting from the Tags 3.0 spec:

The Jakarta Standard Tag Library i18n-capable formatting actions support either approach: They assist page authors with > creating internationalized page content that can be localized into any locale available in the Jakarta Server Pages container (this addresses the second approach), and allow various data elements such as numbers, currencies, dates and times to be formatted and parsed in a locale-sensitive or customized manner (this may be used in either approach).

The Jakarta Standard Tag Library’s i18n actions are covered in this chapter. The formatting actions are covered in Formatting Actions: I18n-capable formatting tag library.

Can Tags 3.0 implementations be required to automatically convert the date values in the JSP source when running on Java 21 (perhaps as per the above pasted Tags 3.0 Spec text)? Or do we need a new Tags Spec release to add an additional requirement? I ask as I think currently we need to do something for EE 11 but at the same time, I also want EE 10 implementations to also be able to pass the Jakarta EE 10 Platform TCK which runs against the same Tags tests.

Also for EE 11, we have a lot of work ahead to refactor the Platform TCK which will include running the Tags TCK tests via Maven + Arquillian, whatever we do to address the challenge for EE 10, will hopefully help with running the EE 11 Platform TCK Tags tests on Java 21 as well as Java 17.

scottmarlow avatar Mar 15 '24 12:03 scottmarlow

I think we should accept this challenge. The issue may not be with JSTL itself / EE11 Specs, but I think we still have to accommodate any changes made in the underlying technology (i.e Java).

@pnicolucci What do you think?

volosied avatar Mar 28 '24 12:03 volosied

I believe the best path forward here is to update the TCK to leverage Java to generate the Date/Time String that is then parsed/formatted using JSTL.

Currently, the TCK has a static String: Nov 21, 2000, 3:45:02 AM used in one of the test failures However, that string can not be parsed by Java 19+ even without Jakarta Tags being involved due to a known change in the JDK.

If the TCK were to use Java to generate the Date/Time values to be tested, then the values would be parsable by Java as they were generated by Java. The TCK could be updated and then the same TCK would work regardless of the JDK version being tested (11/17/21). When running on Java 21 a NNBS would be output, when running on Java 17 a standard space would be output.

Take this code for example:

// TCK uses: Nov 21, 2000, 3:45:02 AM
// Same time in milliseconds: 974778302000
DateFormat dateFormat = DateFormat.getDateTimeInstance();
dateFormat.setTimeZone(TimeZone.getTimeZone(ZoneId.of("GMT")));
String dateFormatZoned = dateFormat.format(new Date(974778302000L) );
pageContext.setAttribute("dateFormatZoned", dateFormatZoned);

The dateFormatZoned attribute now contains the properly formatted String: Nov 21, 2000, 3:45:02 AM with the correct space. The TCK can then perform the same actions it has previously and no issues should be seen when switching between Java versions.

As for the golden files a different file could be used depending on the version of Java being used so that the correct space character could be checked.

pnicolucci avatar Mar 28 '24 13:03 pnicolucci

Posted to the mailing list the latest status: https://www.eclipse.org/lists/jstl-dev/msg00138.html I plan to accept this challenge at the end of the week if there are no objections. @scottmarlow does the above solution seem reasonable to you?

pnicolucci avatar Mar 28 '24 13:03 pnicolucci

Well, I collided with this today when I executed tags TCK for any GF 7.0.x version. Finally I have found that by mistake I executed these tests with JDK21 - that breaks 30 tests of 542. Part of the issue is that TCK is sensitive to the default locale, but that can be resolved by export LC_ALL=C.

With JDK11 and JDK17 GF7 passed all tests of JEE10 TCK.

I know this issue is rather for JEE11, but I would like to have a firm ground first and have everything repeatable and solid in GF7 too. That is why I created https://github.com/eclipse-ee4j/glassfish/pull/24925 - meanwhile I am preparing builds with TCK (private on OmniFish infrastructure as we have more powerful and reliable hw) and documenting the state of GF8 and JEE11 (here, I started by copy pasting JEE 10 and I am slowly going through the list).

In GF master branch you can reproduce it by running this:

mvn clean install -Pfastest,staging -T4C && mvn clean install -Ptck -pl :jakarta-pages-tags-tck && mvn clean install -Ptck -pl :glassfish-external-tck-pages-tags

For GF8 I have to build the TCK first, but the tckrefactor branch build fails because of missing snapshot dependencies. But this problem is the same.

Would be a solution just not depend on default system locale? It is weird when the communication with the user/client depends on server operating system settings. Is that required by the spec?

dmatej avatar Apr 19 '24 17:04 dmatej

@dmatej If you set LC_ALL=C it passes on SE 21? I think that's what you're saying, but it's not clear to me.

bstansberry avatar Apr 19 '24 18:04 bstansberry

@dmatej If you set LC_ALL=C it passes on SE 21? I think that's what you're saying, but it's not clear to me.

No, it doesn't, but it fixes some issues when I have cs_CZ.UTF-8 set on my system. I believe the tck should ensure it.

I did not find any way to pass on JDK21.

dmatej avatar Apr 19 '24 20:04 dmatej

Posted to the mailing list the latest status: https://www.eclipse.org/lists/jstl-dev/msg00138.html I plan to accept this challenge at the end of the week if there are no objections. @scottmarlow does the above solution seem reasonable to you?

@pnicolucci it seems reasonable to me but I'd like to hear from others in the community in the next day or so if anyone else is for/against the stated solution.

I think that we will need a pull request for the https://github.com/jakartaee/platform-tck/tree/10.0.x branch so that we can apply the stated solution. If anyone later challenges the solution as not being valid and that later challenge is accepted, the tests would be excluded instead.

scottmarlow avatar Apr 22 '24 14:04 scottmarlow

@pnicolucci a day has passed and no feedback yet.

Regarding the above solution that you mentioned, do you want to include that for a future EE release? Or for Jakarta EE 10?

My preference is to exclude the tests as I know that could happen this week.

scottmarlow avatar Apr 23 '24 18:04 scottmarlow

We will merge the https://github.com/jakartaee/platform-tck/pull/1219 change as soon as possible that will exclude the failing tests.

scottmarlow avatar Apr 24 '24 18:04 scottmarlow

@scottmarlow how long do you think it would actually take to update the tests rather than exclude them? What sort of time window is available to make test updates?

pnicolucci avatar Apr 24 '24 21:04 pnicolucci

@scottmarlow how long do you think it would actually take to update the tests rather than exclude them? I'm not sure but if I were to work on that, I wouldn't be able to work on something else and I am currently on the critical path for helping to refactor the Platform TCK for the EE 11 release. The same is true for all contributors I think but if someone has the time to make the change, then it takes as long as it takes them to do that while balancing all of their other duties and work. So, hard to say really. I think the guidance in your comment https://github.com/jakartaee/tags/issues/255#issuecomment-2025211892 is enough for someone to work on this.

What sort of time window is available to make test updates?

The current TCK Process suggests the time window should be quick but if we exclude every TCK test, we will end up with zero tests so I think there should be a balance. Also, if we think that most potential contributors are likely not going to have time, that could be a reason to accept the excluding of the tests. Having said this, I think you as lead of the Tags project should decide whether the tests should be fixed or excluded. You could make that choice with other Tags committers or whomever you like. It is perfectly fine for you to make that decision as well.

For TCK challenge resolution, I try to delegate to the respective Spec API lead for the test bucket that contains the challenged test.

Below is some guidance from the TCK Process. Only the first paragraph is relevant here but I included the other details which covers making test changes in response to a challenge:

Accepted Challenges A consensus that a test produces invalid results will result in the exclusion of that test from certification requirements, and an immediate update and release of an official distribution of the TCK including the new exclude list. The associated challenge issue MUST be closed with an accepted label to indicate it has been resolved.

The specification project may approve (user) workarounds for an accepted TCK challenge (as an alternative to excluding TCK tests).

As another alternative to excluding a challenged test, it may be possible to adjust the test validation logic to “expand” the validation check. E.g. if a test expects a value “A1” for a specific variable “x”, but a challenge is raised arguing that the specification language actually allows for either values “A1” and “A2” (but no other values) to be valid values for “x”, then it could be a valid course of action to modify the challenged test to allow either “A1” OR “A2” for “x”.

Since this line of thinking might be applied to cases that aren’t quite as straightforward as this example, care should be taken when using this approach. A particular danger is that an implementation that has already demonstrated compliance before the challenge was raised might actually not pass the new, modified test.

To limit the confusion and additional work such a scenario would cause, if there is already at least one certified compatible implementation before the challenge, the new, modified TCK should be run against at least one such implementation (and ideally all of them) before the changes are published, released, and finalized.

If such a change were released, and it was later found to cause a previously-certified implementation to fail the new, modified test, then excluding the test would likely be the only option, and this would require yet another, additional service release.

scottmarlow avatar Apr 25 '24 17:04 scottmarlow

@scottmarlow I don't have any experience with running the TCK at all, is there any guidance there? What percentage of tests does this challenge account for?

pnicolucci avatar Apr 25 '24 17:04 pnicolucci

@scottmarlow I don't have any experience with running the TCK at all, is there any guidance there?

We plan to make the process of running TCK tests easier for Jakarta EE 11. Some Jakarta EE implementations (e.g. GlassFish + Tomitribe have open sourced their TCK porting kit but I don't think anyone else has so for Open Liberty you likely would need bits that are behind the IBM firewall (same for WildFly currently as well).

If you (or someone else) comes up with a Platform TCK pull request, the Platform TCK team can run tests against your branch. We would build your branch and stage a Platform TCK for testing. We could also stage a Standalone Tags TCK for testing as well. We would likely post on the pull request or here with the TCK results of running the Tags tests with Java 21.

What percentage of tests does this challenge account for?

Currently, with WildFly I am seeing that the Jakarta EE 10 Platform TCK runs 541 tests. If we exclude the 29 tests then there would be 29 fewer tests run but I think the decision might take into account how important the 29 tests are in terms of whether they are testing Tags Specification requirements that are hard to implement correctly. The how to make that judgement of difficulty to implement is very subjective.

scottmarlow avatar Apr 25 '24 17:04 scottmarlow

Just to note, I'll probably build/stage a https://www.eclipse.org/downloads/download.php?file=/jakartaee/platform/10/jakarta-jakartaeetck-10.0.5.zip tomorrow with a Persistence TCK test exclusion that has been outstanding for a while. So if the decision for Tags becomes to exclude the 29 tests by tomorrow, I'll include those changes as well. No pressure either way, just wanted to mention my plans. :-)

scottmarlow avatar Apr 25 '24 18:04 scottmarlow

Just to note, I'll probably build/stage a https://www.eclipse.org/downloads/download.php?file=/jakartaee/platform/10/jakarta-jakartaeetck-10.0.5.zip tomorrow with a Persistence TCK test exclusion that has been outstanding for a while. So if the decision for Tags becomes to exclude the 29 tests by tomorrow, I'll include those changes as well. No pressure either way, just wanted to mention my plans. :-)

Actually, I will wait to release the Platform TCK until we have a fix for https://github.com/jakartaee/tags/issues/255 merged in. I will instead just release the Standalone Persistence TCK today.

scottmarlow avatar Apr 26 '24 14:04 scottmarlow

I'll try running again with -Djava.locale.providers=COMPAT which is mentioned in https://bugs.openjdk.org/browse/JDK-8304925 as a possible workaround.

scottmarlow avatar May 01 '24 17:05 scottmarlow

-Djava.locale.providers=COMPAT didn't help.

scottmarlow avatar May 01 '24 18:05 scottmarlow

@pnicolucci do you know of anyone that has started coding a fix for addressing this issue?

scottmarlow avatar May 07 '24 18:05 scottmarlow

Hi @scottmarlow , I was able to get the TCK running this morning after a couple of days of working with it. Let me try to get something in place now regarding a solution to this problem. I'm sure I'll have TCK-specific questions so I'll post those here as I have them.

pnicolucci avatar May 07 '24 18:05 pnicolucci

@scottmarlow @pnicolucci - We don't have a release planned for Tags in EE 11. As per TCK process, the service release of TCK [ https://github.com/jakartaee/platform-tck/pull/1219 ] allow for exclusion of tests or adjust the test validation logic to “expand” the validation check.I don't believe we are expanding validation with the fix for the challenge here.

gurunrao avatar May 08 '24 06:05 gurunrao