Port Failing Old TCK Tests to New TCK Framework (HTMLUnit & Selenium) #1794

volosied · 2023-02-23T21:14:58Z

Testing still required for MyFaces, but things look to pass on Mojarra.

Tried to keep as true to the original tests as possible.

Ajax
https://github.com/jakartaee/faces/blob/3fae98234692ec16545a6d27cf36fabaeb883f9b/tck/old-tck/source/src/com/sun/ts/tests/jsf/spec/ajax/tagwrapper/URLClient.java

CommandLink
https://github.com/jakartaee/faces/blob/18876c6a72cadc43aecbab9ca8bcebce470c99e3/tck/old-tck/source/src/com/sun/ts/tests/jsf/spec/render/commandlink/URLClient.java

Protected View
https://github.com/jakartaee/faces/blob/3fae98234692ec16545a6d27cf36fabaeb883f9b/tck/old-tck/source/src/com/sun/ts/tests/jsf/spec/view/protectedview/URLClient.java

Notable mentions:

ajaxTagWrappingTest - needed intext.sendKeys(TAB); to trigger change event
viewProtectedViewNonAccessPointTest Checks for ProtectedViewException in the output"
clinkRenderDecodeTest - Can't access header info in selenium so I created a result field
ajaxTagWrappingTest - Original test doesn't check for "true" in the checkedvalue element (neither does the new HTMLUnit test. Chnage was never registered, so the test kept failing), but I added a check to the selenium run.

Lastly, let me know if I should add better error messages?

…work setup

volosied · 2023-02-23T21:16:32Z

These two commits might be the most useful to see the changes:

Original to HTMLUnit:
d126425

HTMLUnit to Selenium:
77f83d5

volosied · 2023-02-23T21:23:29Z

...andLink/src/main/java/com/sun/ts/tests/jsf/spec/render/commandlink/SimpleActionListener.java

@@ -31,11 +31,15 @@
 import jakarta.faces.event.ActionListener;
 import jakarta.servlet.http.HttpServletResponse;

- @jakarta.inject.Named("ActionListener") @jakarta.enterprise.context.SessionScoped
+ @jakarta.inject.Named("ActionListener") @jakarta.enterprise.context.RequestScoped


When I tested this app manually, the result was already set in the later test attempts. So, I changed to RequestScoped. I can change it back, but it shouldn't affect the test.

tck/old-tck-selenium/commandLink/pom.xml

volosied · 2023-02-24T01:30:37Z

tck/old-tck-selenium/ajax/src/main/webapp/tagwrapper/ajaxTagWrap.xhtml

+        "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<!--
+
+    Copyright (c) 2013, 2022 Oracle and/or its affiliates. All rights reserved.


Should copyrights be updated since these application files didn't change much (or very little, i.e. added id)

brideck · 2023-02-24T08:29:44Z

The selenium versions worked fine for me running with OL + MyFaces RC5.

A couple updates you should make:

Update tck/pom.xml to include the new old-tck-selenium module
Update the new HTMLUnit test implementations to extend ITBase and remove the @RunWith annotation. These changes will allow them to be correctly skipped when running with the test.selenium.only property.

volosied · 2023-03-02T13:34:54Z

@werpu The Deployments files don't have a copyright. What should I add? EPL? Apache?

werpu · 2023-03-02T16:08:18Z

@volosied It probably ought to be EPL but others might have a better idea. I know you can mix ASF2 code into an EPL project, but not vice versa, dual licensing also is possible, if it is new code or all authors agree! But I am not an expert on this!

volosied · 2023-03-02T16:53:41Z

Let me ask @waynebeaton to chime in here, maybe he could provide some inside.

Hi Wayne, Werner provided a PR to provide selenium based tests in his PR, but the Deployments classes were missing a copyright. For example, here: https://github.com/jakartaee/faces/pull/1770/files#diff-e4305caf242dfd8b4efa86ae007d0a804cf8538f602acc0dae0bdd0b36b43588

I'm adding new tests and therefore am reusing the selenium test code pattern. I kept the Deployments files copyright-less.

Werner's other code used Apache v2, so I'm not sure what it should be categorized under. Could you comment? Or could it be left as is?

Thank you.

werpu · 2023-03-02T17:04:37Z

Well just a short explanation, I just used ASL2 in code I programmed (aka selenium base framework code), because I knew it was forward compatible with the ECL and I wanted to keep the door open to integrate the same or similar code in myfaces. (In fact I think I even dual licensed itI)
If this door does not need to be kept open I would recommend to go with ECL, but please correct me if I am wrong!

volosied · 2023-03-02T18:28:36Z

I've gone ahead and added EPL to all Deployement files.

waynebeaton · 2023-03-02T19:04:13Z

Please make sure that in all cases where it is technically possible, copyright and license headers are included. Please work together to add the missing headers.

In case there is any confusion... for an Eclipse project, the copyright holder is either the author themselves or -- in the case where an employment contract states -- their employer (we do not require a copyright assignment).

It's generally easier for everybody if contributions are accepted under the same terms as the rest of the project. In the case of Jakarta Faces, that EPL-2.0 or GPL-2.0-only with Classpath-exception-2.0. Consistency makes it harder to make mistakes. Consistency makes it easier for future contributors to understand the terms under which they are contributing (and this gets weirder when individual files are under different licenses).

Consistency also helps avoid some of the "license maths"... by adding a file under Apache-2.0, you would effectively make the license of the entirety of the content (EPL-2.0 or GPL-2.0-only with Classpath-exception-2.0) and Apache-2.0.

Ultimately, though, it's @werpu's intellectual property, and as the copyright holder, his decision to choose the license. If, as a committer, you need assistance to determine whether or not a particular combination of licenses works in a particular scenario, connect with the IP Team.

TL;DR: My strong recommendation is that we get confirmation from @werpu that his contributions were made under the terms of the project license and then add headers that reflect that.

pnicolucci · 2023-03-02T19:14:07Z

Well just a short explanation, I just used ASL2 in code I programmed (aka selenium base framework code), because I knew it was forward compatible with the ECL and I wanted to keep the door open to integrate the same or similar code in myfaces. (In fact I think I even dual licensed itI) If this door does not need to be kept open I would recommend to go with ECL, but please correct me if I am wrong!

@werpu are you saying you're ok with changing the ASL2 copyright to EPL 2.0 for files you initially contributed: https://github.com/jakartaee/faces/pull/1770/files ?

werpu · 2023-03-03T06:27:36Z

@pnicolucci
Absolutely, you have my full permission, since the code which is under ASL2 is 100% written by me, you can change it however you want, the history of being ASL2 first is there, which is good enough!
I will change the license of those files accordingly, i have a pull request pending anyway, I can do it there!
(it is 10 files only anyway)

werpu · 2023-03-03T07:19:22Z

Hi I have relicensed the code i donated to EPL, so that the licencing is more clear, in my pending PR!
I will have the same codebase hosted somewhere else under ASL2 or MIT license to be more liberal. As long as the code is written by me I can do that, to my understanding, however donations by other developers, I probably have to get permission to have this code relicensed. Should not be a big problem as far as I can see it, given the small amount of code affected!
I also will relicense the code for the cleanup task which has yet to be filed as PR, as well!

pnicolucci · 2023-03-03T11:15:14Z

@werpu pending PR with copyright updates: #1795

pnicolucci · 2023-03-03T11:18:57Z

@werpu , see: jakartaee/cdi-tck#437 any objection to donating your same classes to the cdi-tck with the Apache License as @brideck has them currently? Just want to make sure we're all set across the board. We hit the same sort of failures in one cdi tck test that required a selenium update.

werpu · 2023-03-03T11:33:08Z

no objection, I wanted to have them in a very liberal license anyway.
However:
I just did a relicensing a few minutes ago, for unifying the license and to clear up some confusion, but my opinion is that base or framework libs never should be in a very restrictive license, so the ideal would be to have them either in MIT or ASL, but given that there were some requests regarding license unfication i did copyright changes into EPL!

how about having a dual MIT/ASL2 and EPL license on those classes? That should fix things once and for good!

I too want this cleared up, either EPL, or ASL2 or dual. We still can change the license of the affected files! My preference would be to keep this for the base lib classes on the liberal side, if EPL is a must, is there anything speaking against a dual license? I just need a guideline here on which license, and in case of dual licensing, I have to check the correct licensing header!

pnicolucci · 2023-03-03T11:58:30Z

@waynebeaton is the dual license ok? Is there someone on the IP team that can help us if you don't know the answer? Thanks! Essentially we need the code now to be at least in the Faces project and the cdi-tck project, which have different licenses.

BalusC · 2023-03-03T12:25:38Z

tck/old-tck-selenium/ajax/src/main/webapp/WEB-INF/faces-config.xml

+<faces-config xmlns="https://jakarta.ee/xml/ns/jakartaee"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartaee/web-facesconfig_3_0.xsd"
+    version="3.0">


Should be Faces 4.0

BalusC · 2023-03-03T12:26:15Z

tck/old-tck-selenium/ajax/src/main/webapp/WEB-INF/web.xml

+    SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+-->
+
+<web-app version="5.0" xmlns="https://jakarta.ee/xml/ns/jakartaee" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartaee/web-app_5_0.xsd">


Should be Servlet 6.0

I copied these from the old tck, but, considering that that ajax tests had three apps, I could only use 1 web.xml (jsf_ajax_jsresource).

I should rename it

Is updating to web.xml to 6.0 and the faces-config to 4.0 appropriate since the older tests used these definitions?

What about the other app web.xmls/configs?

CDI 4.0 was declared in beans.xml which implied JEE 10.

So either make sure all deployment descriptors are JEE 10 (CDI 4.0, Faces 4.0, Servlet 6.0) or make sure all deployment descriptors are JEE 9 (CDI 3.0, Faces 3.0, Servlet 5.0). If the old test didn't have a beans.xml then probably you should downgrade beans.xml to CDI 3.0. But all XHTML files are using the new XML namespaces introduced in Faces 4.0. So it wouldn't work in Faces 3.0 nonetheless.

What about the other app web.xmls/configs?

All deployment descriptors should preferably be in sync with each other. Originally the deployment descriptors should follow the target Faces version the test was written for. E.g. if the test is testing something which was fixed in e.g. Faces 2.3 then all DD files should be conform JEE 8 because it needs to be testable against Faces 2.3 as well (although this does maybe not apply to TCK itself).

JEE 10: CDI 4.0 + Faces 4.0 + Servlet 6.0

JEE 9: CDI 3.0 + Faces 3.0 + Servlet 5.0

JEE 8: CDI 2.0 + Faces 2.3 + Servlet 4.0

JEE 7: CDI 1.2 + Faces 2.2 + Servlet 3.1

JEE 6: CDI 1.0 + Faces 2.0 + Servlet 3.0

volosied · 2023-03-03T15:21:11Z

tck/old-tck-selenium/commandLink/src/main/webapp/WEB-INF/faces-config.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
-    Copyright (c) 2009, 2018 Oracle and/or its affiliates. All rights reserved.
+    Copyright (c) 2005, 2023 Oracle and/or its affiliates. All rights reserved.


2005 is the correct year:

faces/tck/old-tck/source/src/com/sun/ts/tests/jsf/spec/render/commandlink/faces-config.xml

Line 4 in 3fae982

Copyright (c) 2005, 2018 Oracle and/or its affiliates. All rights reserved.

volosied · 2023-03-03T15:23:51Z

I'd like to squash the commits before merging, once the approvals are set.

waynebeaton · 2023-03-03T15:31:23Z

@waynebeaton is the dual license ok?

Yes.

Is there someone on the IP team that can help us if you don't know the answer?

I run the IP Team, so yeah... I know some people...

pnicolucci · 2023-03-03T16:36:24Z

@waynebeaton is the dual license ok?

Yes.

Is there someone on the IP team that can help us if you don't know the answer?

I run the IP Team, so yeah... I know some people...

Thanks @waynebeaton :)

Can you help us with what a dual license header would look like for EPL 2.0 and ASL 2.0? Just want to make sure we get it correct on the first try! We need to merge the same code here in the Faces project which currently is SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0` as well in the cdi-tck https://github.com/jakartaee/cdi-tck/blob/master/LICENSE. Link to the cdi-tck PR: jakartaee/cdi-tck#437 Whatever header we use would need to be the same for both projects or would there be a difference?

waynebeaton · 2023-03-03T16:50:39Z

In keeping with the established expression, you can use something like this:

(EPL-2.0 or GPL-2.0 with Classpath-exception-2.0) and Apache-2.0

Note that the GPL-2.0 identifier has been deprecated by SPDX. I've used it in my suggestion to be consistent with the rest of the headers that I see in the project. There is no requirement to update existing license expressions.

If you're rather use a current code, use GPL-2.0-only.

pnicolucci · 2023-03-03T17:09:14Z

@waynebeaton , ok so for the purpose of the Faces Community here in this PR we would just put the following header in this new code:

/*
 * Copyright (c) 2023 Contributors to the Eclipse Foundation.
 *
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License v. 2.0, which is available at
 * http://www.eclipse.org/legal/epl-2.0.
 *
 * This Source Code may also be made available under the following Secondary
 * Licenses when the conditions for such availability set forth in the
 * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
 * version 2 with the GNU Classpath Exception, which is available at
 * https://www.gnu.org/software/classpath/license.html.
 *
 * SPDX-License-Identifier: (EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0) and Apache-2.0
 */

No other updates other than: SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 to: SPDX-License-Identifier: (EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0) and Apache-2.0

For the cdi-tck we could then just contribute the same code with the ASL 2.0 license header?

/**
* Copyright Werner Punz
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*     http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

Correct or did I mess it up ? :)

waynebeaton · 2023-03-03T17:53:25Z

No. I messed up. I misread the question and only gave a part answer. Gimme a minute.

waynebeaton · 2023-03-03T18:07:31Z

My apologies. It turns out that I also got the expression and operator wrong. It should or not and. Thanks for verifying.

Try this for the dual licensing scenario.

/*
 * Copyright (c) 2023 Contributors to the Eclipse Foundation.
 *
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License v. 2.0 which is available at
 * https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
 * which is available at https://www.apache.org/licenses/LICENSE-2.0.
 * 
 * This Source Code may also be made available under the following Secondary
 * Licenses when the conditions for such availability set forth in the Eclipse
 * Public License v. 2.0 are satisfied: GPL-2.0 with Classpath-exception-2.0 which 
 * is available at https://openjdk.java.net/legal/gplv2+ce.html.
 * 
 * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 or Apache-2.0
 */

The header for the Apache license looks right.

pnicolucci · 2023-03-03T18:22:26Z

Thanks so much, @waynebeaton . Ok so... @werpu @volosied Wayne has provided a copyright header for you guys to update your PRs. @brideck the cdi PR is good since you already have the Apache License in that PR.

werpu · 2023-03-03T18:57:50Z

Yes I will do that in the next hour, @waynebeaton, thank you!
changed headers are pushed in my PR now

...aces22/ajax/src/test/java/ee/jakarta/tck/faces/test/servlet30/ajax_selenium/Deployments.java

Skip ported tests Update copyrights for new files Address review comments correct packages in skipped tests Correct more copyrights Add EPL copyrights to Deployments.java files' Update to EE10 deployment descriptors Update to EPLv2 / Apache License v2.0 More copyright update, mention Apache v2

volosied · 2023-03-06T02:36:14Z

I believe this PR should be ready now that the last copyrights were fixed.

Please review and merge when possible!

pnicolucci · 2023-03-06T12:04:04Z

@BalusC @arjantijms any other comments?

volosied added 5 commits February 23, 2023 15:24

inital app port for ajax, commandlink, and protectedview + test frame…

b773091

…work setup

Port original tests and helper methods

d8b70ac

Convert to pass with available HTMLUnit

d126425

Copy-Paste tests to selenium class file to easily view differences

26710e7

Convert tests to use selenium

77f83d5

volosied commented Feb 23, 2023

View reviewed changes

volosied commented Feb 24, 2023

View reviewed changes

tck/old-tck-selenium/commandLink/pom.xml Outdated Show resolved Hide resolved

volosied commented Feb 24, 2023

View reviewed changes

volosied force-pushed the old-tck-selenium-port-final branch from 1707a17 to 532a1ec Compare February 24, 2023 14:37

pnicolucci added this to the 4.0.1-TCK milestone Feb 26, 2023

pnicolucci requested review from arjantijms, BalusC, pnicolucci, dmatej, brideck and tandraschko March 2, 2023 02:12

brideck approved these changes Mar 2, 2023

View reviewed changes

volosied mentioned this pull request Mar 2, 2023

Several additional Tests with Ajax dependencies, adding Selenium implementation! #1795

Merged

pnicolucci mentioned this pull request Mar 3, 2023

Add Support for Selenium Tests jakartaee/cdi-tck#437

Merged

BalusC reviewed Mar 3, 2023

View reviewed changes

volosied commented Mar 3, 2023

View reviewed changes

volosied force-pushed the old-tck-selenium-port-final branch from e03f61e to 784d630 Compare March 3, 2023 15:23

pnicolucci reviewed Mar 5, 2023

View reviewed changes

...aces22/ajax/src/test/java/ee/jakarta/tck/faces/test/servlet30/ajax_selenium/Deployments.java Show resolved Hide resolved

volosied force-pushed the old-tck-selenium-port-final branch from f65a11d to f6562cc Compare March 6, 2023 02:34

pnicolucci approved these changes Mar 6, 2023

View reviewed changes

Add skipped tests to ts.jtx.platform

51286e3

volosied force-pushed the old-tck-selenium-port-final branch from c2952e8 to 51286e3 Compare March 7, 2023 00:52

pnicolucci merged commit 89617af into jakartaee:master Mar 7, 2023

arjantijms modified the milestones: 4.0.1-TCK, 4.0.2-TCK Mar 10, 2023

Port Failing Old TCK Tests to New TCK Framework (HTMLUnit & Selenium) #1794

Port Failing Old TCK Tests to New TCK Framework (HTMLUnit & Selenium) #1794

Conversation

volosied commented Feb 23, 2023 • edited Loading

volosied commented Feb 23, 2023

volosied Feb 23, 2023

Choose a reason for hiding this comment

volosied Feb 24, 2023

Choose a reason for hiding this comment

brideck commented Feb 24, 2023 • edited Loading

volosied commented Mar 2, 2023

werpu commented Mar 2, 2023 • edited Loading

volosied commented Mar 2, 2023

werpu commented Mar 2, 2023

volosied commented Mar 2, 2023

waynebeaton commented Mar 2, 2023

pnicolucci commented Mar 2, 2023

werpu commented Mar 3, 2023 • edited Loading

werpu commented Mar 3, 2023 • edited Loading

pnicolucci commented Mar 3, 2023

pnicolucci commented Mar 3, 2023 • edited Loading

werpu commented Mar 3, 2023 • edited Loading

pnicolucci commented Mar 3, 2023 • edited Loading

BalusC Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

BalusC Mar 3, 2023

Choose a reason for hiding this comment

volosied Mar 3, 2023

Choose a reason for hiding this comment

volosied Mar 3, 2023

Choose a reason for hiding this comment

BalusC Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

BalusC Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

volosied Mar 3, 2023

Choose a reason for hiding this comment

volosied commented Mar 3, 2023

waynebeaton commented Mar 3, 2023

pnicolucci commented Mar 3, 2023

waynebeaton commented Mar 3, 2023

pnicolucci commented Mar 3, 2023

waynebeaton commented Mar 3, 2023

waynebeaton commented Mar 3, 2023

pnicolucci commented Mar 3, 2023

werpu commented Mar 3, 2023 • edited Loading

volosied commented Mar 6, 2023 • edited Loading

pnicolucci commented Mar 6, 2023

volosied commented Feb 23, 2023 •

edited

Loading

brideck commented Feb 24, 2023 •

edited

Loading

werpu commented Mar 2, 2023 •

edited

Loading

werpu commented Mar 3, 2023 •

edited

Loading

werpu commented Mar 3, 2023 •

edited

Loading

pnicolucci commented Mar 3, 2023 •

edited

Loading

werpu commented Mar 3, 2023 •

edited

Loading

pnicolucci commented Mar 3, 2023 •

edited

Loading

BalusC Mar 3, 2023 •

edited

Loading

BalusC Mar 3, 2023 •

edited

Loading

BalusC Mar 3, 2023 •

edited

Loading

werpu commented Mar 3, 2023 •

edited

Loading

volosied commented Mar 6, 2023 •

edited

Loading