Tidy up session tests, add more comments.

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractClientCrossContextSessionTest.java

@@ -26,6 +26,7 @@ import org.eclipse.jetty.client.ContentExchange;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.http.HttpMethods;
 import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
 import org.junit.Test;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -46,10 +47,14 @@ public abstract class AbstractClientCrossContextSessionTest
         String contextB = "/contextB";
         String servletMapping = "/server";
         AbstractTestServer server = createServer(0);
+        TestServletA servletA = new TestServletA();
+        ServletHolder holderA = new ServletHolder(servletA);
         ServletContextHandler ctxA = server.addContext(contextA);
-        ctxA.addServlet(TestServletA.class, servletMapping);
+        ctxA.addServlet(holderA, servletMapping);
         ServletContextHandler ctxB = server.addContext(contextB);
-        ctxB.addServlet(TestServletB.class, servletMapping);
+        TestServletB servletB = new TestServletB();
+        ServletHolder holderB = new ServletHolder(servletB);
+        ctxB.addServlet(holderB, servletMapping);
         server.start();
         int port = server.getPort();
         
@@ -80,6 +85,7 @@ public abstract class AbstractClientCrossContextSessionTest
                 client.send(exchangeB);
                 exchangeB.waitForDone();
                 assertEquals(HttpServletResponse.SC_OK,exchangeB.getResponseStatus());
+                assertEquals(servletA.sessionId, servletB.sessionId);
             }
             finally
             {
@@ -94,11 +100,17 @@ public abstract class AbstractClientCrossContextSessionTest
 
     public static class TestServletA extends HttpServlet
     {
+        public String sessionId;
+        
         @Override
         protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
         {
             HttpSession session = request.getSession(false);
-            if (session == null) session = request.getSession(true);
+            if (session == null)
+            {
+                session = request.getSession(true);
+                sessionId = session.getId();
+            }
 
             // Add something to the session
             session.setAttribute("A", "A");
@@ -106,17 +118,22 @@ public abstract class AbstractClientCrossContextSessionTest
             // Check that we don't see things put in session by contextB
             Object objectB = session.getAttribute("B");
             assertTrue(objectB == null);
-            System.out.println("A: session.getAttributeNames() = " + Collections.list(session.getAttributeNames()));
         }
     }
 
     public static class TestServletB extends HttpServlet
     {
+        public String sessionId;
+        
         @Override
         protected void doGet(HttpServletRequest request, HttpServletResponse httpServletResponse) throws ServletException, IOException
         {
             HttpSession session = request.getSession(false);
-            if (session == null) session = request.getSession(true);
+            if (session == null)
+            {
+                session = request.getSession(true);
+                sessionId = session.getId();
+            }
 
             // Add something to the session
             session.setAttribute("B", "B");
@@ -124,7 +141,6 @@ public abstract class AbstractClientCrossContextSessionTest
             // Check that we don't see things put in session by contextA
             Object objectA = session.getAttribute("A");
             assertTrue(objectA == null);
-            System.out.println("B: session.getAttributeNames() = " + Collections.list(session.getAttributeNames()));
         }
     }
 }

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractImmortalSessionTest.java

@@ -44,6 +44,7 @@ public abstract class AbstractImmortalSessionTest
         String contextPath = "";
         String servletMapping = "/server";
         int scavengePeriod = 2;
+        //turn off session expiry by setting maxInactiveInterval to -1
         AbstractTestServer server = createServer(0, -1, scavengePeriod);
         server.addContext(contextPath).addServlet(TestServlet.class, servletMapping);
         server.start();

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractLastAccessTimeTest.java

@@ -98,13 +98,11 @@ public abstract class AbstractLastAccessTimeTest
                         Thread.sleep(requestInterval);
                     }
 
-                    System.out.println("Waiting for scavenging on node1...");
-
                     // At this point, session1 should be eligible for expiration.
                     // Let's wait for the scavenger to run, waiting 2.5 times the scavenger period
                     Thread.sleep(scavengePeriod * 2500L);
 
-                    // Access again server1, and be sure we can
+                    // Access again server1, and ensure that we can still access the session
                     exchange1 = new ContentExchange(true);
                     exchange1.setMethod(HttpMethods.GET);
                     exchange1.setURL("http://localhost:" + port1 + contextPath + servletMapping);
@@ -112,6 +110,8 @@ public abstract class AbstractLastAccessTimeTest
                     client.send(exchange1);
                     exchange1.waitForDone();
                     assertEquals(HttpServletResponse.SC_OK, exchange1.getResponseStatus());
+                    //test that the session was kept alive by server 2 and still contains what server1 put in it
+                    assertEquals("test", exchange1.getResponseContent());
                     
                 }
                 finally

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractOrphanedSessionTest.java

@@ -84,19 +84,7 @@ public abstract class AbstractOrphanedSessionTest
                     // must be removed by scavenging done in the other node.
                     Thread.sleep(TimeUnit.SECONDS.toMillis(inactivePeriod + 2L * scavengePeriod));
 
-                    System.err.println("FINISHED waiting for session to expire");
                     // Perform one request to server2 to be sure that the session has been expired
-//                    
-
-                    // force invalidate to test
-//                    ContentExchange exchange3 = new ContentExchange(true);
-//                    exchange3.setMethod(HttpMethods.GET);
-//                    exchange3.setURL("http://localhost:" + port2 + contextPath + servletMapping + "?action=remove");
-//                    exchange3.getRequestFields().add("Cookie", sessionCookie);
-//                    client.send(exchange3);
-//                    exchange3.waitForDone();
-                    
-                    System.err.println("CHECKING NODE2");
                     ContentExchange exchange2 = new ContentExchange(true);
                     exchange2.setMethod(HttpMethods.GET);
                     exchange2.setURL("http://localhost:" + port2 + contextPath + servletMapping + "?action=check");

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractReentrantRequestSessionTest.java

@@ -92,7 +92,7 @@ public abstract class AbstractReentrantRequestSessionTest
                 int port = Integer.parseInt(request.getParameter("port"));
                 String path = request.getParameter("path");
 
-                // We want to make another request with a different session
+                // We want to make another request 
                 // while this request is still pending, to see if the locking is
                 // fine grained (per session at least).
                 try