-
Notifications
You must be signed in to change notification settings - Fork 55
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
Prepare new Faces TCK #1600
Comments
|
I can help, the question is how to partition the work. |
@starksm64 easiest I think by just taking ownership of one of the items. If you can mention the item here, e.g. 1, then everyone knows you'll be looking at that one. |
Ok. |
@arjantijms for 1, do you want that test2 module added as a new tck module? |
@starksm64 Indeed, the regular /tck folder. This TCK btw does not have the kind of split that Jakarta REST has. We still need to see what the best way is for vendors to add their own connector. |
Ok, I'll take item 1. and create a PR for it. |
Should the tests be passing? I have copied test over and made the group name consistent, but I see the first test that are run failing. I do see the same com.sun.faces.test.javaee7.cdibeanvalidator.Issue3014IT fail in the mojarra/test2 main brach as well: [INFO] -------------------------------------------------------
[INFO] Running com.sun.faces.test.javaee7.cdibeanvalidator.Issue3014IT
Starting container using command: [/Library/Java/JavaVirtualMachines/jdk-11.0.13.jdk/Contents/Home/bin/java, -jar, /Users/starksm/Dev/JBoss/Jakarta/rh-faces/target/glassfish7/glassfish/modules/admin-cli.jar, start-domain, -t]
Successfully started the domain : domain1
domain Location: /Users/starksm/Dev/JBoss/Jakarta/rh-faces/target/glassfish7/glassfish/domains/domain1
Log File: /Users/starksm/Dev/JBoss/Jakarta/rh-faces/target/glassfish7/glassfish/domains/domain1/logs/server.log
Admin Port: 4848
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 12.132 s <<< FAILURE! - in com.sun.faces.test.javaee7.cdibeanvalidator.Issue3014IT
[ERROR] com.sun.faces.test.javaee7.cdibeanvalidator.Issue3014IT.testValidatorInjection Time elapsed: 1.074 s <<< FAILURE!
java.lang.AssertionError
at com.sun.faces.test.javaee7.cdibeanvalidator.Issue3014IT.testValidatorInjection(Issue3014IT.java:70) Stopping container using command: [/Library/Java/JavaVirtualMachines/jdk-11.0.13.jdk/Contents/Home/bin/java, -jar, /Users/starksm/Dev/JBoss/Jakarta/rh-faces/target/glassfish7/glassfish/modules/admin-cli.jar, stop-domain, -t] |
That's "correct", as Hibernate Validator still hasn't released a Jakarta EE 10 compatible version, so Bean Validation doesn't work at all in GlassFish 7. See https://hibernate.atlassian.net/browse/HV-1857 a little nudge to them would sure help I think ;) |
Ok, I have added the dependencies that need to be updated to the HV-1857 issue. You will need to push out the Index of org/glassfish:jakarta.el:5.0.0-M1 staging implementation of EL 5.0.0 as well. |
Signed-off-by: Scott M Stark <starksm64@gmail.com>
Now I see 2, to change the package names. I'll do that in the current #1601 PR. So is the change com.sun.faces.* -> ee.jakarta.tck.faces.*? |
Indeed, |
Signed-off-by: Scott M Stark <starksm64@gmail.com>
Ok, item 2 is also in the #1601 PR |
Copy the mojarra tck into the faces project, #1600 item 1
Hi Arjan - I wanted to clarify on this part and the overall goals for faces TCK in JakartaEE 10. Is it the expectation that a new Faces TCK should be built from this repo for JakartaEE10 release cycle? If so, are all tests(or the working ones) from jakartaee-tck repo(src/com/sun/ts/tests/jsf) need to be migrated to this repo as JUnit/Arquillian tests ? In old-style tck tests(from jakartaee-tck) , currently most tests does not work due to compilation issues after the removal of deprecated methods in the api, I am working to fix them. I was looking to get help for converting jakarta.faces.bean.ManagedBean to CDI bean here. |
This is probable way too much work for this cycle, as it concerns thousands of tests (I don't know how Jakarta REST/JAX-RS did this). My idea was a little among the following:
WDYT? Please note that for the old-style TCK the existing *.jsp files won't work anymore either, and those would have to be converted to .xhtml, which means extra work on top of the already big task. |
JAXRS has less than half the number of Faces tests. It required relatively fewer changes than what is required for faces now.
IIUC this means we will use the new JUnit/Maven based TCK as the main TCK bundle within which we can invoke the old-style TCK tests. I think the old tests(source & classes) that are run also need to be part of the TCK bundle.
Agree. |
In that example, yes, but it's added locally to Maven via an other pom, which is this one: If they eventually need to be packaged into a single bundle that should be doable. |
Hey guys, what's the status of this? Looks like (1) and (2) are complete? |
@kito99 Yes, 1 and 2 have been done. I added a checkmark to them. Now it's a matter of starting to add assertion IDs, specifically to https://github.com/jakartaee/faces/tree/master/tck/faces40 |
This would require below sub tasks to be complete in jakartaee-tck. a. Fix the build issues in the current source as reported in jakartaee/platform-tck#807. This is fixed and under review jakartaee/platform-tck#815 . |
I will work on 5 & 6. Help would be appreciated to update the TCK Userguide contents for 4.0.0 after they are moved to this repo. |
@arjantijms excuse my ignorance, but what is an assertion ID? |
The assertions take some literal requirement from either the spec or the javadoc and assign an ID to it. This ID is then used in tests. They are listed in a file, here the rendered HTML versions: https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/install/jsf/docs/assertions There's often a source .xml too, but sometimes those are lost or duplicated. It's a bit messy at times, unfortunately. For source files see here: https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/internal/docs/jsf (@alwin-joseph maybe those need some cleaning up?) Here you see them used in the actual tests: https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/jsf/api/jakarta_faces/application/application/URLClient.java#L61 |
(p.s. the assertion files are maybe a reflection of the "mapping file" thinking which was very popular in the late 90s and mid 00s. Maybe the current culture and thinking has more shifted towards tagging them at the location where they are defined, e.g. directly in the spec document and the javadoc) |
That sounds like a pretty good idea. I proposed the other approach mostly because I thought it would be easier/faster to do, but having the actual files here and not have that dependency would indeed be better. |
Created PR #1649 for migrating the faces test sources from jakartaee-tck. |
@kito99 sorry to bother again, just wondering if you already have been able to find some time with helping to fix the outstanding TCK issues. We're done to 1001 failures, and every fix helps. |
@alwin-joseph I just merged #1649 @BalusC can you look to see if you can get the old TCK building / running? |
@tandraschko do you have time to try to run the TCK (old and new) against MyFaces? |
It would be great if we can generate the new TCK bundle via jenkins job(in faces Jenkins CI) and make it available at a download location so we can review the structure/contents and make amends if required. |
@alwin-joseph Indeed, that should be doable for sure. @BalusC @tandraschko @kito99 @mnriem Did any of you succeeded in building the old TCK as transferred to this project? Not talking about fixing or running anything yet, just trying to build it. Nothing more. Hopefully just a single person has this 10 to 20 minutes to spare. |
It's quite unclear how to build the old TCK, but this one gives an example for expresion language: jakartaee/platform-tck#933 (comment) I'll copy it here:
Example of installing the TCK after building it: Example of running the TCK: https://github.com/eclipse-ee4j/glassfish/blob/master/appserver/tests/tck/pages/pom.xml cc @dmatej |
|
Sounds great Alwin, thanks! Now maybe the only thing needed there is the ability to deploy and run a single test. I added that for Pages here: https://github.com/eclipse-ee4j/glassfish/blob/master/appserver/tests/tck/pages/pom.xml#L261 and here: https://github.com/eclipse-ee4j/glassfish/blob/master/appserver/tests/tck/pages/pom.xml#L295 @BalusC with that new info from Alwin, can you continue on? |
Will https://ci.eclipse.org/faces be used to build the new Faces TCK? Have any implementations started to create any public CI jobs for running the Faces TCK? |
@scottmarlow not yet, I'm mostly waiting for some more of the old tests to be fixed. But I understand working with the (old) TCK is not so easy for everyone, even if those people could help with the Faces work itself. |
Hi, I'd like to help out with Faces and the TCK here. I'm trying to follow this conversation here. Are we trying to fix the old tck and then move these tests over to the new Faces TCK? Can anyone give me any tasks? (@arjantijms) Thanks! Update:
|
Thanks a bunch, much appreciated!
The first task is to simply build and run the old TCK. Almost nobody (AFAIK) has been able to do that. It's perhaps a trivial task, but someone needs to do it. There were 1001 failures when last time executed when still in the platform TCK, though typically it aren't 1001 unique failures. So the next obvious task would be to start reducing those failures. A second (simultaneous) task would be to make it possible to run a single test, like e.g. can be done here: https://github.com/eclipse-ee4j/glassfish/blob/master/appserver/tests/tck/pages/pom.xml |
@arjantijms TCK bundle is available at https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee10/staged/eftl/jakarta-faces-tck-4.0.0-SNAPSHOT.zip |
PR #1653 should address 1, 3, and 5 in #1600 (comment) |
Below are the mvn targets that can be used for new tck build/run/generation. 1. To build and run the new tck :
This will first start building the old tck hence takes longer time to start running the new tests. 2. To build and run the old tck alone :
3. To generate the new TCK bundle :
The job https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-faces-tck-build/ does the same, copies the bundle at https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee10/staged/eftl/jakarta-faces-tck-4.0.0-SNAPSHOT.zip. 4. TCK run using the bundle:
Currently this is broken due to 2 reasons :
|
@volosied @arjantijms The latest test result of old tck run from repository :
https://ci.eclipse.org/jakartaee-tck/job/10/job/jakarta-faces-tck-build-run/14/consoleText |
Remaining failures:
I will look at updating the application/resource & application/resourcewrapper test next via #1663. Tests will need to be updated to expect the correct URLs. I think the elresolvers tests should also be looked at next. |
@volosied @BalusC @tandraschko @alwin-joseph As our time is essentially up, and Faces needs to go in ballot tomorrow, I think we'd best start to pack-up what we have now, excluding the small list of tests above. Perhaps we can do a maintenance update including the final tests shortly after EE 10? |
Created #1666 to exclude the remaining 25 tests that fails. IMO the TCK userguide need to be reviewed/updated and final TCK packaging and structure need to be revisited as I had faced below issue earlier.
Will the TCK build and run jobs be created in faces Jenkins? |
Yes, I've done this for Security al ready, and the plan is to do the same for Faces and Authentication. For Security, see this job: https://ci.eclipse.org/es/job/2_security-run-tck-against-staged-build/ #!/usr/bin/env groovy
node {
def API_JAR_NAME = "jakarta.security.enterprise-api.jar"
def DOWNLOAD_API_JAR_NAME = "jakarta.security.enterprise-api-2.0.0.jar"
def API_URL = "https://jakarta.oss.sonatype.org/content/repositories/staging/jakarta/security/enterprise/jakarta.security.enterprise-api/2.0.0/${DOWNLOAD_API_JAR_NAME}"
def IMPL_JAR_NAME = "jakarta.security.enterprise.jar"
def DOWNLOAD_IMPL_JAR_NAME = "jakarta.security.enterprise-2.0.1.jar"
def IMPL_URL = "https://jakarta.oss.sonatype.org/content/repositories/staging/org/glassfish/soteria/jakarta.security.enterprise/2.0.1/${DOWNLOAD_IMPL_JAR_NAME}"
def IMPL_JAR_NAME2 = "soteria.spi.bean.decorator.weld.jar"
def DOWNLOAD_IMPL_JAR_NAME2 = "soteria.spi.bean.decorator.weld-2.0.1.jar"
def IMPL_URL2 = "https://jakarta.oss.sonatype.org/content/repositories/staging/org/glassfish/soteria/soteria.spi.bean.decorator.weld/2.0.1/${DOWNLOAD_IMPL_JAR_NAME2}"
def GF_URL = "https://ci.eclipse.org/glassfish/view/GlassFish/job/glassfish_build-and-test-using-jenkinsfile/job/master/lastSuccessfulBuild/artifact/bundles/glassfish.zip"
def TCK_BUNDLE_NAME = "jakarta-security-tck-3.0.0"
def TCK_BUNDLE_URL = "https://download.eclipse.org/es/jakartaee10/staged/eftl/${TCK_BUNDLE_NAME}.zip"
env.ANT_VERSION='1.9.15'
env.TOOLS_PREFIX='/opt/tools'
env.JAVA_PREFIX="${TOOLS_PREFIX}/java/openjdk"
env.MVN_HOME="${TOOLS_PREFIX}/apache-maven/latest"
env.JAVA_HOME="${JAVA_PREFIX}/jdk-17/latest"
env.PATH="${MVN_HOME}/bin:${JAVA_HOME}/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
env.ANT_OPTS="-Djavax.xml.accessExternalSchema=all"
env.JAVA_TOOL_OPTIONS ='-XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockExperimentalVMOptions '
stage("Init") {
// https://go.cloudbees.com/docs/cloudbees-documentation/cjoc-user-guide/index.html#cluster-copy-artifacts
dir ("download") {
// If any artefacts from other jobs need to be used, they can be placed here
// copyRemoteArtifacts(from: "${GF_BUILD_JOB}")
// copyRemoteArtifacts(from: "${TS_JTE_BUILD_JOB}")
}
sh '''#!/bin/bash -x
mkdir download
'''
}
stage("Grab GF") {
env.GF_URL = "${GF_URL}"
sh '''#!/bin/bash -ex
# Add support for downloading GF from an arbitrary location
'''
}
stage("Grab API and IMPL") {
env.API_URL = "${API_URL}"
env.API_JAR_NAME = "${API_JAR_NAME}"
env.IMPL_URL = "${IMPL_URL}"
env.IMPL_JAR_NAME = "${IMPL_JAR_NAME}"
env.IMPL_URL2 = "${IMPL_URL2}"
env.IMPL_JAR_NAME2 = "${IMPL_JAR_NAME2}"
sh '''#!/bin/bash -ex
'''
}
stage("Grab TCK") {
env.TCK_BUNDLE_URL = "${TCK_BUNDLE_URL}"
env.TCK_BUNDLE_NAME = "${TCK_BUNDLE_NAME}"
sh '''#!/bin/bash -ex
cd ${WORKSPACE}/download
wget -q ${TCK_BUNDLE_URL} -O ${TCK_BUNDLE_NAME}.zip
cd ${WORKSPACE}
unzip ${WORKSPACE}/download/${TCK_BUNDLE_NAME}.zip
cat ${TCK_BUNDLE_NAME}/pom.xml
'''
}
stage ("Configure ts.jte") {
sh '''#!/bin/bash -ex
'''
}
stage("Configure TCK") {
sh '''#!/bin/bash -ex
'''
}
stage ("Run TCK tests") {
env.TCK_BUNDLE_NAME = "${TCK_BUNDLE_NAME}"
sh '''#!/bin/bash -x
ls -altrh
echo ${TCK_BUNDLE_NAME}
cd ${TCK_BUNDLE_NAME}
mvn clean install -Pstaging,glassfish-ci-managed -Dglassfish.version=7.0.0-M4 surefire-report:failsafe-report-only -Daggregate=true | tee ${WORKSPACE}/run.log
'''
}
stage ("Create summary.txt, API, and run.log artifacts") {
env.TCK_BUNDLE_NAME = "${TCK_BUNDLE_NAME}"
sh '''#!/bin/bash -ex
cd ${WORKSPACE}/${TCK_BUNDLE_NAME}
# Should not be needed, but some reason is.
mvn site -Pstaging -pl -:glassfish-external-tck-security surefire-report:failsafe-report-only -Daggregate=true -DskipITs=true -DskipSurefireReport=true -Dglassfish.version=7.0.0-M4 -T2
cd ${WORKSPACE}
ls -altrh
ls ${TCK_BUNDLE_NAME}
ls ${WORKSPACE}/${TCK_BUNDLE_NAME}
ls ${WORKSPACE}/${TCK_BUNDLE_NAME}/target
cat ${TCK_BUNDLE_NAME}/target/site/failsafe-report.html | sed '/table/,/table/!d;//d' | sed '/Package/q' | sed -n 's:.*<td.*>\\(.*\\)</td>.*:\\1:p' > tmp_result.txt
PASSED_COUNT=`cat tmp_result.txt | sed '1q;d'`
ERROR_COUNT=`cat tmp_result.txt | sed '2q;d'`
FAILED_COUNT=`cat tmp_result.txt | sed '3q;d'`
echo "********************************************************************************" >> summary.txt
echo "Completed running ${PASSED_COUNT} tests." >> summary.txt
echo "Number of Tests Failed = ${FAILED_COUNT}" >> summary.txt
echo "Number of Tests with Errors = ${ERROR_COUNT}" >> summary.txt
echo "********************************************************************************" >> summary.txt
cat run.log | sed -e '1,/Completed running/d' >> summary.txt
SHA256_API=`sha256sum ${WORKSPACE}/${TCK_BUNDLE_NAME}/target/glassfish7/glassfish/modules/${API_JAR_NAME} | awk '{print $1}'`
SHA256_IMPL=`sha256sum ${WORKSPACE}/${TCK_BUNDLE_NAME}/target/glassfish7/glassfish/modules/${IMPL_JAR_NAME} | awk '{print $1}'`
SHA256_TCK=`sha256sum ${WORKSPACE}/download/${TCK_BUNDLE_NAME}.zip | awk '{print $1}'`
echo ERROR_COUNT=${ERROR_COUNT}
echo FAILED_COUNT=${FAILED_COUNT}
echo PASSED_COUNT=${PASSED_COUNT}
echo ----
echo SHA256_API=${SHA256_API} | tee -a summary.txt
echo SHA256_IMPL=${SHA256_IMPL} | tee -a summary.txt
echo SHA256_TCK=${SHA256_TCK} | tee -a summary.txt
echo ----
echo TCK_download=${TCK_BUNDLE_URL} | tee -a summary.txt
echo ----
OS1=`lsb_release -a || true`
OS2=`cat /etc/issue.net || true`
OS3=`cat /etc/*_version || true`
OS4=`cat /etc/*-release || true`
if [ ! -z "${OS1}" ]; then
echo OS1=${OS1} | tee -a summary.txt
fi
if [ ! -z "${OS2}" ]; then
echo OS2=${OS2} | tee -a summary.txt
fi
if [ ! -z "${OS3}" ]; then
echo OS3=${OS3} | tee -a summary.txt
fi
if [ ! -z "${OS4}" ]; then
echo OS4=${OS4} | tee -a summary.txt
fi
JDK_VERSION=$(java -version 2>&1 || true)
echo JDK_VERSION=${JDK_VERSION} | tee -a summary.txt
'''
archiveArtifacts artifacts: "summary.txt", fingerprint: true
archiveArtifacts artifacts: "run.log", fingerprint: true
}
}
Maybe we can do a minor maintenance release of the TCKs after EE 10. E.g. a 4.0.1 for security to clean up everything a little. |
Should these tasks be carried into a Faces 4.1 TCK task list and this issue closed? |
At a glance I don't think anything needs to be carried over. These are all very specific items from the old to new TCK migration. I'll just close the issue now then. |
Prepare a new Faces TCK by
ee.jakarta.tck
(see Describe steps necessary to help out with TCK eclipse-ee4j/mojarra#5046)
Where needed, add assertion IDs(on hold, nobody understands this)(see Prepare new Faces TCK #1600 (comment))
(For example, see https://github.com/arjantijms/glassfish/blob/master/appserver/tests/tck/faces/pom.xml)
(see https://github.com/piranhacloud/piranha/tree/current/external/tck/jwt)
cc @BalusC @scottmarlow @mriem
The text was updated successfully, but these errors were encountered: