[TCK Challenge] A number of cross context HTTP session tests are failing

[mnriem]

The Piranha project delivers a Servlet container that returns “null”  for the ServletContext.getContext(String uripath) which is allowable as per the Servlet 6.0 JavaDoc.

As the tests in question assume that a ServletContext is returned they fail.

1. com/sun/ts/tests/servlet/api/jakarta_servlet_http/httpsessionx/URLClient.java#invalidateHttpSessionxTest
2. com/sun/ts/tests/servlet/api/jakarta_servlet_http/httpsessionx/URLClient.java#expireHttpSessionxrfTest
3. com/sun/ts/tests/servlet/api/jakarta_servlet_http/httpsessionx/URLClient.java#expireHttpSessionxri1Test
4. com/sun/ts/tests/servlet/api/jakarta_servlet_http/httpsessionx/URLClient.java#expireHttpSessionxriTest

[mnriem]

@markt-asf What is the status of this issue?

[markt-asf]

The test can be excluded for now and we'll fix it for 6.1.

[janbartel]

There are a lot more cross-context tests, shouldn't these also be fixed?:

com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.dispatchAfterCommitTest4 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.dispatchAfterCommitTest5 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.dispatchReturnTest4 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.dispatchReturnTest5 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.negativeDispatchTest12 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.negativeDispatchTest13 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.negativeDispatchTest8 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.negativeDispatchTest9 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.startAsyncAgainTest12 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.startAsyncAgainTest13 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.startAsyncAgainTest14 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.startAsyncAgainTest15 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.startAsyncAgainTest16 com/sun/ts/tests/servlet/api/jakarta_servlet/dispatchtest/URLClient.java.startAsyncAgainTest17

[gregw]

Should the tests be modified to succeed if they get a null back from ServletContext.getContext(String), but if it get's a non null return then it goes on to do the actual test? Or do we need some other way to configure the tests to say that a container is expected to implement cross context dispatch? @olamy thoughts?

[mnriem]

If all these tests also fail for the same reason they are valid TCK challenges and they should be excluded for the current TCK. @markt-asf Can you acknowledge this?

[arjantijms]

Should the tests be modified to succeed if they get a null back from ServletContext.getContext(String), but if it get's a non null return then it goes on to do the actual test?

If I remember the specification text correctly, that should be the expected behaviour. ServletContext.getContext(String) can return null, and that's perfectly fine. But if it returns anything non-null, things should be as specified.

[mnriem]

@arjantijms But for the current TCK this means those tests are currently invalid and should be excluded, now? That is the procedure, right?

[olamy]

@gregw I would prefer to adapt the test to keep running for container implementing cross context. But in the case of Jetty (in the tests mentioned by @janbartel here), ServletContext.getContext(String) doesn't return but AsyncContext.dispatch(...) generate 404. So we cannot rely on the fact ServletContext.getContext(String) is returning null to keep running the test or not. Maybe we should have some configuration saying container support cross context or not. This could be achieve by a System property. Or maybe ServletContext.getContext(String) returning null should/could be this flag (so we have something to fix in Jetty 12 ;))

[gregw]

Or maybe ServletContext.getContext(String) returning null should/could be this flag (so we have something to fix in Jetty 12 ;))

Perhaps calling servletContent.getContext(ServletContext.getContextPath()) would be a good way to test if the container supports cross context dispatch or not?

[olamy]

Or maybe ServletContext.getContext(String) returning null should/could be this flag (so we have something to fix in Jetty 12 ;))

Perhaps calling servletContent.getContext(ServletContext.getContextPath()) would be a good way to test if the container supports cross context dispatch or not?

yes I definitely like the idea. if any objections in the next few days. I will propose PRs to TCK code. Others?

[mnriem]

So for the current 6.0 TCK this means those tests are to be excluded, right?

[olamy]

So for the current 6.0 TCK this means those tests are to be excluded, right?

For 6.x we have options:

1. adapt current TCK 6.x code to check if servletContent.getContext(...) as a marker then skip the rest of crosscontext tests so container implementing this will have full set of testing
2. simply ignore them

No strong opinions on which option (1. might be a good amount of code changes)

For tckrefactor maybe option 1.?

[mnriem]

@markt-asf As you have already pushed other TCK tests similar to this to a TCK 6.1 release can we exclude these for 6.0 TCK as well?

[markt-asf]

I agree these tests can be excluded for 6.0 and we'll need to amend them for 6.1 so the exclusion can be removed.

[olamy]

implemented here https://github.com/jakartaee/servlet/pull/553

[mnriem]

@olamy @markt-asf Can we close this out?