Update jstl dates for java21

pnicolucci commented 1 month ago

Fixes Issue This is for Jakarta Tags Challenge: https://github.com/jakartaee/tags/issues/255

Related Issue(s) https://github.com/jakartaee/tags/issues/255

Describe the change I'm working through the failing sub buckets to update golden files for Java pre and post Java 19. This is a work in progress as I have a couple more sub buckets to work through but I wanted to get a PR up to show progress.

I've decided to leave the original JSPs alone and just create a new set of JSPs and golden files where necessary. This is done where ever we have AM/PM in the actual JSP. If there is no AM/PM in the JSP and we just need to work with the new space character I've just modified the existing JSP to UTF-8 encoding and added a new golden file.

There are more clever ways to do this but I wanted to limit the changes to the existing code base with the intention of this change running on both EE10 and EE11 and being conscious of the fact that these changes are to address a challenge. We can make more clever changes in the future if we have to address a similar problem.

A few additional comments:

positivePDDateStyleTest / positivePDTimeStyleTest / positivePDParseLocaleTest all have an existing goldenfile but the golden file is not used within the JSTLClient for that test so I didn't touch these goldenfiles.
positiveFDTimeZoneNullEmptyTest / positivePDTimeZoneNullEmptyTest were both spelled incorrectly so I fixed the spelling.
If I removed the STANDARD or STANDARD_COMPAT property so that I could override the goldenfiles then I added the TEST_NAME property.
For the new JSPs that were created they are just copies of the original JSP with updates to spacing and formatting.
There are a couple tests that required additional formatting changes from the original in addition to just the space between AM and PM, mainly full date formats in some instance us a , rather than at such as: Friday, December 26, 1997 at 7:11:34 PM Pacific Standard Time in the original and Friday, December 26, 1997, 7:11:34 PM Pacific Standard Time in the updated golden file. These golden files include: onedotzero\positiveSetTimezoneValueTestJava20Plus.gf onedotzero\positiveTimezoneValueTestJava20Plus.gf timezone\positiveTimezoneValueNullEmptyTestJava20Plus.gf timezone\positiveTimezoneValueTestJava20Plus.gf settimezone\positiveTimezoneValueNullEmptyTestJava20Plus.gf settimezone\positiveTimezoneValueTestJava20Plus.gf

Additional context See the Jakarta Tags Challenge.

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

pnicolucci commented 1 month ago

Latest update, I have one more sub bucket to work through "parsedate" which I'll work to finish later today.

scottmarlow commented 1 month ago

Latest update, I have one more sub bucket to work through "parsedate" which I'll work to finish later today.

@pnicolucci thanks for the effort, very much appreciated!

scottmarlow commented 1 month ago

fyi, I started a build of the EE 10 Platform TCK with a copy of your branch via https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/TAGSTEST/1 which will take a few hours to build. Once you have further changes, I'll build that again so that others can test with your changes.

https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/TAGSTEST/lastSuccessfulBuild/artifact/jakartaeetck-bundles/jakartaeetck-11.0.0.zip has the Platform TCK build based on changes from this morning. This is an EE 10 Platform TCK archive.

I'll build the standalone JSTL tck next via https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/TAGSTEST/2/

The JSTL tck is at https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/TAGSTEST/2/artifact/standalone-bundles/tags-tck-3.0.0.zip

scottmarlow commented 1 month ago

https://gist.github.com/scottmarlow/deaf4263fc5633849cbcf3a3d56a71b5 has my local test results and three remaining failures that I see (hopefully the same as your I have one more sub bucket to work through task). I'll build the TCK again when you have further changes.

Note that I changed into the jstl/compat/onedotzero (really jakartaeetck/src/com/sun/ts/tests/jstl/compat/onedotzero) to run only the subset of tests that are relevant for this challenge.

pnicolucci commented 1 month ago

https://gist.github.com/scottmarlow/deaf4263fc5633849cbcf3a3d56a71b5 has my local test results and three remaining failures that I see (hopefully the same as your I have one more sub bucket to work through task). I'll build the TCK again when you have further changes.

Note that I changed into the jstl/compat/onedotzero (really jakartaeetck/src/com/sun/ts/tests/jstl/compat/onedotzero) to run only the subset of tests that are relevant for this challenge.

@scottmarlow could you run the entire Tags TCK once I have a final PR up, hoping today or sometime tomorrow. It isn't correct that onedotzero is the only bucket that fails with Java 21, see your initial challenge here: https://github.com/jakartaee/tags/issues/255

pnicolucci commented 1 month ago

@scottmarlow do you have any more detailed logs for your execution? I would have expected those onedotzero tests to pass they pass locally for me with Java 17 and Java 21.

scottmarlow commented 1 month ago

@scottmarlow do you have any more detailed logs for your execution? I would have expected those onedotzero tests to pass they pass locally for me with Java 17 and Java 21.

https://gist.github.com/scottmarlow/f8c64ca3eebc08d9c61c414898a66902 has one of the failures.

@scottmarlow could you run the entire Tags TCK once I have a final PR up, hoping today or sometime tomorrow. It isn't correct that onedotzero is the only bucket that fails with Java 21, see your initial challenge here: jakartaee/tags#255

Thanks, I will run the other tests now as well.

pnicolucci commented 1 month ago

@scottmarlow one other thing what java version are you executing the tests with? I'm just curious if there is an issue with how I'm parsing the java string potentially.

scottmarlow commented 1 month ago

@scottmarlow one other thing what java version are you executing the tests with? I'm just curious if there is an issue with how I'm parsing the java string potentially.

I'm using:

openjdk version "21.0.3" 2024-04-16 OpenJDK Runtime Environment (Red_Hat-21.0.3.0.9-1) (build 21.0.3+9) OpenJDK 64-Bit Server VM (Red_Hat-21.0.3.0.9-1) (build 21.0.3+9, mixed mode, sharing)

Updated test results:

Test results: passed: 528; failed: 13

Next I'll try the latest OpenJDK from https://download.java.net/java/GA/jdk21.0.2/f2283984656d49d69e91c558476027ac/13/GPL/openjdk-21.0.2_linux-x64_bin.tar.gz

openjdk version "21.0.2" 2024-01-16 OpenJDK Runtime Environment (build 21.0.2+13-58) OpenJDK 64-Bit Server VM (build 21.0.2+13-58, mixed mode, sharing)

Update: Output of String version = System.getProperty("java.version") for me locally is 21 regardless of whether I use the Red Hat or download.java.net/java/GA/jdk21.0.2 build of Java 21.

Update: With the jdk21.0.2 download, I still see the same failures Test results: passed: 528; failed: 13 which is fewer failures than I saw before but still more than you are getting.

scottmarlow commented 1 month ago

I suppose it could be a problem with the https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow job as well. If the tests are passing for you and all of your changes are included in the pull request, I think we can do an official build from https://ci.eclipse.org/jakartaee-tck/job/10/job/eftl-jakartaeetck-build-100/ after the change is merged (further changes can be made if needed after).

pnicolucci commented 1 month ago

I've pushed up changes for the last sub bucket. I need to do some cleanup before this is ready for review. I'll work to do that tonight and tomorrow then we should be ready for some additional testing / debug to finalize this PR.

pnicolucci commented 1 month ago

@scottmarlow I think the reason the onedotzero tests are still failing is due to a build issue, I had some problems getting things to build locally for this, if we continue to see this and everything else ends up passing with this PR I'll see if I can solve that as well!

pnicolucci commented 1 month ago

@scottmarlow for the build issue, I guess before I spend too much time on it lets see if an actual build works? I'll document the issue I saw here. Usually to build one of the applications I would for example go to:

1) jakartaeetck/src/com/sun/ts/tests/jstl/compat/onedotzero 2) execute the following ant command: ant compile build package 3) My app would be updated and I can execute a set of tests with my changes.

However in the onedotzero there is another target so I had to do the following: ant compile build pakcage.compat package

https://github.com/jakartaee/platform-tck/blob/a2cb4419366cf6a846d20e4925d642fd5a3c8795/src/com/sun/ts/tests/jstl/compat/onedotzero/build.xml#L48

So if this target isn't being invoked by the build then the app won't be updated. If we continue to see these onedotzero tests failing with this changes we'll need to check the dist and verify the changes are actually reflected in the built application.

scottmarlow commented 1 month ago

I started a Platform TCK build of your changes via https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/TAGSTEST/4/ which is building via my (just updated) fork of your branch at https://github.com/scottmarlow/jakartaee-tck/commits/TAGSTEST/

pnicolucci commented 1 month ago

@scottmarlow I think we're ready for another test :)

Another set of eyes on the changes would be welcomed as well. I can give it another look over to make sure it all looks good and I didn't miss copyrights etc.

<edit> I see you already started!! </edit>

scottmarlow commented 1 month ago

Updated: Thanks for the above information. I updated my local script to follow your advise and then removed my compilation of the jstl source which I don't need at the moment as far as I understand.

Never mind about the validation error that I mentioned as that isn't the cause of the failure I am seeing. Will let the tests continue to run and report results later.

scottmarlow commented 1 month ago

I am currently seeing three failures with Java 21:

com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveFormatLocalizationContextI18NTest Unexpected 404 received from target server! Request path: /jstl_1_0_compat_web/positiveFormatLocalizationContextI18NTestJava20Plus.jsp
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveSetTimezoneValueTest See https://gist.github.com/scottmarlow/e8b3cb5107aa3d09c5ca7eb0baf2c414 for test output
com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveTimezoneValueTest See https://gist.github.com/scottmarlow/c6218cbc5106afe46f274a903a0b8a73 for test output

For reference, I get the failures whether or not I recompile the tests with the following:

if [[ "$testFolder" == testFolder=jstl* ]]; then cd $TS_HOME/src/com/sun/ts/tests/$testFolder ant compile build package.compat package cd $TS_HOME/src/web/jstl ant compile build package fi

With Java 17, all of the jstl tests pass.

pnicolucci commented 1 month ago

Hey @scottmarlow , ok so down to just the one old sub bucket not working. Can you pass me the built WAR from your test output so I can try it out locally and see what's happening?

Also, do these tests fail for you on Java 21 without my changes?

I'm thinking the web application isn't actually being updated but the JSTLClient is which would explain the 404 for the first test case you listed.

scottmarlow commented 1 month ago

Hey @scottmarlow , ok so down to just the one old sub bucket not working. Can you pass me the built WAR from your test output so I can try it out locally and see what's happening?

jstl_1_0_compat_web.war.zip is really jstl_1_0_compat_web.war and is copied from my local jakartaeetck/dist/com/sun/ts/tests/jstl/compat/onedotzero

Also, do these tests fail for you on Java 21 without my changes?

Yes, these three tests do fail for me on Java 21 without your changes.

I'm thinking the web application isn't actually being updated but the JSTLClient is which would explain the 404 for the first test case you listed.

In case it helps, the ^ jstl_1_0_compat_web.war.zip is from dist/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war:

ls -l dist/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war -rwxrwxrwx. 1 smarlow smarlow 2181309 May 30 09:51 dist/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war

I also seem to have src/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war:

ls -l src/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war -rw-rw-rw-. 1 smarlow smarlow 41433 May 29 16:07 src/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war

scottmarlow commented 1 month ago

From locally debugging the https://github.com/jakartaee/platform-tck/blob/master/src/com/sun/ts/tests/common/webclient/validation/TokenizedValidator.java#L112C22-L112C45 while running the com/sun/ts/tests/jstl/compat/onedotzero/JSTLClient.java#positiveSetTimezoneValueTest, I see:

Expected value from golden file: 7:11:34â¯PM Actual result returned from WildFly: 7:11:34?PM

scottmarlow commented 1 month ago

@pnicolucci should we create a jakartaeetck/src/web/jstl/compat/onedotzero/positiveSetTimezoneValueTest.jsp that has the NBSP characters in it like positivePDTypeTestJava20Plus.jsp and some others?

This case may be different though. The goldenfile contains:

positiveTimezoneValueTest Friday, December 26, 1997, 7:11:34â¯PM Pacific Standard Time Friday, December 26, 1997, 7:11:34â¯PM Pacific Standard Time Friday, December 26, 1997, 7:11:34â¯PM Pacific Standard Time Friday, December 26, 1997, 7:11:34â¯PM Pacific Standard Time Friday, December 26, 1997, 7:11:34â¯PM Pacific Standard Time Saturday, December 27, 1997, 2:11:34â¯AM GMT-01:00

And the response contains:

positiveTimezoneValueTest Friday, December 26, 1997, 7:11:34?PM Pacific Standard Time Friday, December 26, 1997, 7:11:34?PM Pacific Standard Time Friday, December 26, 1997, 7:11:34?PM Pacific Standard Time Friday, December 26, 1997, 7:11:34?PM Pacific Standard Time Friday, December 26, 1997, 7:11:34?PM Pacific Standard Time Saturday, December 27, 1997, 2:11:34?AM GMT-01:00

So it seems like this is a different case as the date comes from the following in the JSP:

Date date = new Date(883192294202L);

pnicolucci commented 1 month ago

@scottmarlow there isn't anyplace to add the NBSP unless I missed something? https://github.com/jakartaee/platform-tck/blob/master/src/web/jstl/compat/onedotzero/positiveSetTimezoneValueTest.jsp ?

This one created a Date using milliseconds. Then the resulting response will contain the NBSP, so I just updated the JSP to utf-8 encoding so the character is rendered properly and a new goldenfile is added.

pnicolucci commented 1 month ago

The WAR here doesn't contain any of the updates from this PR: https://github.com/jakartaee/platform-tck/files/15501534/jstl_1_0_compat_web.war.zip

scottmarlow commented 1 month ago

The WAR here doesn't contain any of the updates from this PR: https://github.com/jakartaee/platform-tck/files/15501534/jstl_1_0_compat_web.war.zip

Good catch. It looks like my custom platform TCK build didn't build jstl_1_0_compat_web.war correctly. I'll use your source changes instead and build them locally similar to what you have done to test.

scottmarlow commented 1 month ago

I'm running from a fresh copy of the Jakarta EE Platform 10.0.2 zip and manually applied the attached tags.patch.txt patch (via patch -p1 < tags.patch in my jakartaeetck folder).

I will try building these changes via:

cd $TS_HOME/src/web/jstl ant compile build package cd $TS_HOME/src/com/sun/ts/tests/$testFolder ant compile build package.compat package

I'll also verify that the generated jstl_1_0_compat_web.war contains the updated jsp files.

scottmarlow commented 1 month ago

@pnicolucci could you please attach a copy of your dist/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war that you are testing with? I'd like to try running with that locally.

scottmarlow commented 1 month ago

@pnicolucci could you please attach a copy of your dist/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war that you are testing with? I'd like to try running with that locally.

@pnicolucci could you please attach a copy of your dist/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war that you are testing with? I'd like to try running with that locally.

I think we need to update the https://github.com/jakartaee/platform-tck/tree/10.0.x/src/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war that is checked in to contain your changes.

pnicolucci commented 1 month ago

@pnicolucci could you please attach a copy of your dist/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war that you are testing with? I'd like to try running with that locally.

I've zipped it up and attached: jstl_1_0_compat_web.zip

pnicolucci commented 1 month ago

@pnicolucci could you please attach a copy of your dist/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war that you are testing with? I'd like to try running with that locally.

@pnicolucci could you please attach a copy of your dist/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war that you are testing with? I'd like to try running with that locally.

I think we need to update the https://github.com/jakartaee/platform-tck/tree/10.0.x/src/com/sun/ts/tests/jstl/compat/onedotzero/jstl_1_0_compat_web.war that is checked in to contain your changes.

I can do that, give me a few and I'll push up an updated WAR, odd it builds for me but not as part of the build process.

I've been building directly in src/com/sun/ts/tests/jstl/compat/onedotzero using the command I previously posted.

I do see the WAR was updated previously: https://github.com/jakartaee/platform-tck/pull/664

pnicolucci commented 1 month ago

@scottmarlow I just pushed up a commit with the updated WAR.

NOTE: If I need to update copyrights, reminder to update the WAR as well with the 3 JSPs that were updated in the commit.

scottmarlow commented 1 month ago

@scottmarlow I just pushed up a commit with the updated WAR.

Thanks, I'm building with that change via https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/TAGSTEST/5 and will test that locally.

NOTE: If I need to update copyrights, reminder to update the WAR as well with the 3 JSPs that were updated in the commit.

Update: WildFly passed the jstl tests locally with Java 21 + https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck-scottmarlow/job/TAGSTEST/5/artifact/jakartaeetck-bundles/jakartaeetck-11.0.0.zip

Your recent update to include the jstl_1_0_compat_web.war made the difference! Thank you again for that!

scottmarlow commented 1 month ago

Update: WildFly is now passing the jstl tests on Java 17 as well.

Test results: passed: 541

java -version openjdk version "17.0.11" 2024-04-16 OpenJDK Runtime Environment (Red_Hat-17.0.11.0.9-1) (build 17.0.11+9) OpenJDK 64-Bit Server VM (Red_Hat-17.0.11.0.9-1) (build 17.0.11+9, mixed mode, sharing)

scottmarlow commented 1 month ago

I do not see any problems other than the few source files that I commented on today regarding the copyright in newly added files.

pnicolucci commented 1 month ago

@scottmarlow the new JSP files are exact copies of the existing JSPs with minor updates, UTF-8 encoding, and an update of the space character for Java 20 Plus. I kept the original copyright since they were copies. I wanted to clarify why I left the original copyright, I didn't feel comfortable removing it since I was copying existing code.

Is the consensus of the platform-tck project that I should remove the original copyright from the new JSPs even if they are copies of existing code?

scottmarlow commented 1 month ago

@scottmarlow the new JSP files are exact copies of the existing JSPs with minor updates, UTF-8 encoding, and an update of the space character for Java 20 Plus. I kept the original copyright since they were copies. I wanted to clarify why I left the original copyright, I didn't feel comfortable removing it since I was copying existing code.

Good point, I knew you were copying existing code. I now think either way would of been okay. I'm going to resolve the my comments.

Is the consensus of the platform-tck project that I should remove the original copyright from the new JSPs even if they are copies of existing code?

scottmarlow commented 1 month ago

Looks good to me, lets give it a few more days before merging in case @alwin-joseph or @gurunrao have feedback.

pnicolucci commented 1 month ago

@scottmarlow thanks very much for all of your help and testing!! Very much appreciated!

pnicolucci commented 4 weeks ago

@scottmarlow This PR is against the master branch. Do I need any other PR for 10.0.x or tckrefactor branches?

alwin-joseph commented 4 weeks ago

Context on Tags tests for EE11:

Tags tests were refactored for EE11 Platform TCK in tckrefactor branch (https://github.com/jakartaee/platform-tck/tree/tckrefactor/jstl) to use Arquillian and Junit.
The tests can be run with Glassfish 8(JDK17 & JDK21) using https://github.com/jakartaee/platform-tck/tree/tckrefactor/glassfish-runner/jstl-tck.

Tags TCK Fix :

IMO the current changes in this PR belongs to 10.0.x branch primarily and the PR should be re-targeted for same.
For tckrefactor branch(EE11) a separate PR will need to be created based on the test failures and fixes required. Currently there were 15 test failures in tckrefactor branch with Glassfish8+JDK21 related to https://github.com/jakartaee/tags/issues/255 due to space character before AM/PM. Other implementations would need to execute this refactored tck to find the results.
IMO if we are to merge tckrefactor with master soon, it would be appropriate to have the fixes in tckrefactor branch and test first.

cc @gurunrao

pnicolucci commented 4 weeks ago

@scottmarlow @alwin-joseph

You don't want these changes in the master branch.
Create this same PR to the 10.0.x branch.
You also want another PR to the tckrefactor branch, I'll need some help here as I'm not familiar with the tests and how they've been rewritten in that branch. Can someone else take a shot at making these same changes to the tckrefactor branch? Are the failures between the tckrefactor branch and the 10.0.x branch the same when running with Java 21?

I'm confused about what branch will eventually be used to test Jakarta Tags in EE11. I thought the intent was to update the 3.0.0 tests so that we can use the same TCK for both EE10 and EE11.

alwin-joseph commented 4 weeks ago

3. You also want another PR to the `tckrefactor` branch, I'll need some help here as I'm not familiar with the tests and how they've been rewritten in that branch. Can someone else take a shot at making these same changes to the `tckrefactor` branch? Are the failures between the `tckrefactor` branch and the `10.0.x` branch the same when running with Java 21?
I can take up this, will create a PR in tckrefactor branch so the same will be included in EE11 Platform TCK. It would certainly help if other implementations can validate the test runs after the fix is applied.

I'm confused about what branch will eventually be used to test Jakarta Tags in EE11. I thought the intent was to update the 3.0.0 tests so that we can use the same TCK for both EE10 and EE11.

The refactored Tags tests in tckrefactor is intended to be included in EE11 Platform TCK and not standalone TCK. For standalone Tags TCK EE11, the 10.0.x branch will be used, same for EE10 and EE11. cc @scottmarlow @gurunrao Please add/correct me if I missed anything.

scottmarlow commented 4 weeks ago

@scottmarlow @alwin-joseph
1. You don't want these changes in the `master` branch.

No need to merge to the master branch. Sorry that I missed this before.

2. Create this same PR to the 10.0.x branch.

Please do. I validated your change as if it was a 10.0.x change.

3. You also want another PR to the `tckrefactor` branch, I'll need some help here as I'm not familiar with the tests and how they've been rewritten in that branch.

Clearly we will need to figure out which Tags tests will be run against Jakarta EE 11 implementations (e.g. Tags 3.0 standalone TCK or EE 11 Platform TCK).

Can someone else take a shot at making these same changes to the tckrefactor branch? Are the failures between the tckrefactor branch and the 10.0.x branch the same when running with Java 21?

Eventually (after EE 11) we will need to make changes to be able to pass the Tags tests against a future EE release but we shouldn't run refactored Tags TCK tests against EE 11 implementations as that wouldn't be valid.

I'm confused about what branch will eventually be used to test Jakarta Tags in EE11. I thought the intent was to update the 3.0.0 tests so that we can use the same TCK for both EE10 and EE11.

Yes, but I think we previously missed the impact on EE 11 implementation testing.

scottmarlow commented 4 weeks ago

We should discuss this further...

scottmarlow commented 4 weeks ago

Updated: I now think that it is fine for the EE 11 Platform TCK to run JSTL tests (based on Tags 3.0) against EE 11 implementations. So in summary, I am +1 for merging this pr to the 10.0.x branch. More inline below:

Context on Tags tests for EE11:

* Tags tests were refactored for EE11 Platform TCK in `tckrefactor` branch (https://github.com/jakartaee/platform-tck/tree/tckrefactor/jstl) to use Arquillian and Junit.

* The tests can be run with Glassfish 8(JDK17 & JDK21) using https://github.com/jakartaee/platform-tck/tree/tckrefactor/glassfish-runner/jstl-tck.

Tags TCK Fix :

* IMO the current changes in this PR belongs to `10.0.x` branch primarily and the PR should be re-targeted for same.

+1

* For `tckrefactor` branch(EE11) a separate PR will need to be created based on the test failures and fixes required. Currently there were 15 test failures in `tckrefactor` branch with Glassfish8+JDK21 related to [Tags 3.0 TCK Challenge against tests that include space character before AM/PM tags#255](https://github.com/jakartaee/tags/issues/255) due to space character before AM/PM. Other implementations would need to execute this refactored tck to find the results.

+1

* IMO if we are to merge `tckrefactor` with `master` soon, it would be appropriate to have the fixes in `tckrefactor` branch and test first.

It would be fine to merge the changes into both 10.0.x + master or just 10.0.x. Personally, I would probably just update this pr to target 10.0.x which should be an easy change since master is almost the same as 10.0.x. The master branch is probably causing confusion from being around still.

I also want to mention that some unrelated changes in the master branch will need to be merged to the tckrefactor before we complete the EE 11 release. More specifically, I am thinking of the https://github.com/jakartaee/platform-tck/pull/1220 changes that moved tests from the CDI TCK to the Platform TCK (via cdi-ee-tck folder).

pnicolucci commented 4 weeks ago

10.0.x PR: https://github.com/jakartaee/platform-tck/pull/1320

pnicolucci commented 2 weeks ago

3. You also want another PR to the `tckrefactor` branch, I'll need some help here as I'm not familiar with the tests and how they've been rewritten in that branch. Can someone else take a shot at making these same changes to the `tckrefactor` branch? Are the failures between the `tckrefactor` branch and the `10.0.x` branch the same when running with Java 21?
I can take up this, will create a PR in tckrefactor branch so the same will be included in EE11 Platform TCK. It would certainly help if other implementations can validate the test runs after the fix is applied.

I'm confused about what branch will eventually be used to test Jakarta Tags in EE11. I thought the intent was to update the 3.0.0 tests so that we can use the same TCK for both EE10 and EE11.

The refactored Tags tests in tckrefactor is intended to be included in EE11 Platform TCK and not standalone TCK. For standalone Tags TCK EE11, the 10.0.x branch will be used, same for EE10 and EE11. cc @scottmarlow @gurunrao Please add/correct me if I missed anything.

@alwin-joseph did you have a chance to get a PR for the tckrefactor branch?

alwin-joseph commented 2 weeks ago

@alwin-joseph did you have a chance to get a PR for the tckrefactor branch?

Not yet. I will get to that soon.

jakartaee / platform-tck

Update jstl dates for java21 #1312