faces
faces copied to clipboard
TCK Challenge: Test assertions should not make implementation-specific assumptions
Challenged Tests: ee.jakarta.tck.faces.test.javaee8.commandScript.Spec613IT#testCommandScript ee.jakarta.tck.faces.test.servlet40.el.Spec1337IT#testResourceEL3 ee.jakarta.tck.faces.test.servlet30.ajax.Issue3097IT#testViewExpired1
TCK Version: Jakarta Faces 4.0.x
Tested Implementation: Open Liberty -- containing MyFaces 4.0
Description: The challenged tests all have assertions that are making assumptions based off of the output that Mojarra produces:
Spec613IT -
The test is checking that the web response contains the string ">var foo=function(o){"
. Unfortunately, MyFaces produces the slightly different output of ">var foo = function(o){"
so the test fails. These are obviously the same string on a fundamental level.
Spec1337IT -
The test is checking that the page contains the phrase "contains more than one colon"
. Not having run the TCK with Mojarra, I can't say exactly what the context of this phrase is, but given the nature of the test this is probably either part of an error message or exception text. MyFaces throws an exception here that clearly indicates that there is a problem with the resource, but does not contain this specific text:
Caused by: jakarta.el.ELException: Malformed resource reference found when resolving resource3EL::resourceEL3.gif
at org.apache.myfaces.el.resolver.ResourceResolver.getValue(ResourceResolver.java:99)
at jakarta.el.CompositeELResolver.getValue(CompositeELResolver.java:62)
at org.apache.el.parser.AstValue.getValue(AstValue.java:169)
at org.apache.el.ValueExpressionImpl.getValue(ValueExpressionImpl.java:190)
at org.apache.myfaces.view.facelets.el.ContextAwareTagValueExpression.getValue(ContextAwareTagValueExpression.java:100)
Issue3097IT -
The test is checking the result page for the string "class jakarta.faces.application.ViewExpiredException"
. With MyFaces, the correct exception (ViewExpiredException) is thrown and surfaced in the result page, but the result does not contain the "class"
keyword, so the test fails. Without knowing the full context of Mojarra's result page it's hard to say why the test writer would have found it necessary to include "class"
in the assertion text. Checking for "jakarta.faces.application.ViewExpiredException"
should be sufficient to verify correct behavior.
These tests need to be rewritten in an implementation-neutral fashion for a future release of the Faces TCK.
From my point of view, this is a valid challenge. @arjantijms @BalusC @tandraschko do you agree?
At a glance it indeed seems valid.
For the current version we should exclude those tests then, or, if reasonably possible, update the tests in such a way that no new or different functionality is tested and then ask for the common exception (contradictio in terminis, I know) again to allow the update.
@BalusC wdyt?
@BalusC any input? Thanks!
Test assertions should not make implementation-specific assumptions
Completely correct.
Spec613IT
Test should be rewritten if window.foo
is available at all and is of type function
(and thus not e.g. string
).
Spec1337IT
Test should be rewritten to check if jakarta.el.ELException
is thrown at all.
Issue3097IT
Test should be rewritten to check if the function behind faces.ajax.addOnError()
is called at all.
@BalusC @arjantijms given the discussion here I think we can accept this challenge.
I can open a new challenge, if desired, but as we peel the onion on some tests blocked by other challenges I've found 2 more tests that would fit here: ee.jakarta.tck.faces.test.javaee8.cdi.Spec527IT#testInjectFacesContext ee.jakarta.tck.faces.test.javaee8.cdi.Spec1309IT#testInjectExternalContext
Both tests are explicitly testing that injected Contexts are the com.sun.faces
implementation, e.g. com.sun.faces.context.ExternalContextImpl
@arjantijms are you ok adding the tests above from @brideck or would you prefer a new challenge?
Addressed by https://github.com/jakartaee/faces/pull/1728 which updated the tests and should complete the challenge.