Merge pull request #2542 from richardTingle/#2541-move-screenshots-to-docker

#2541 move screenshots to docker

.github/workflows/main.yml

@@ -59,46 +59,41 @@ jobs:
   ScreenshotTests:
     name: Run Screenshot Tests
     runs-on: ubuntu-latest
+    container:
+      image: ghcr.io/onemillionworlds/opengl-docker-image:v1
     permissions:
       contents: read
     steps:
-    - uses: actions/checkout@v4
-    - name: Set up JDK 17
-      uses: actions/setup-java@v4
-      with:
-        java-version: '17'
-        distribution: 'temurin'
-    - name: Install Mesa3D
-      run: |
-        sudo apt-get update
-        sudo apt-get install -y mesa-utils libgl1-mesa-dri libgl1 libglx-mesa0 xvfb
-    - name: Set environment variables for Mesa3D
-      run: |
-        echo "LIBGL_ALWAYS_SOFTWARE=1" >> $GITHUB_ENV
-        echo "MESA_LOADER_DRIVER_OVERRIDE=llvmpipe" >> $GITHUB_ENV
-    - name: Start xvfb
-      run: |
-        sudo Xvfb :99 -ac -screen 0 1024x768x16 &
-        export DISPLAY=:99
-        echo "DISPLAY=:99" >> $GITHUB_ENV
-    - name: Verify Mesa3D Installation
-      run: |
-        glxinfo | grep "OpenGL"
-    - name: Validate the Gradle wrapper
-      uses: gradle/actions/wrapper-validation@v3
-    - name: Test with Gradle Wrapper
-      run: |
-        ./gradlew :jme3-screenshot-test:screenshotTest
-    - name: Upload Test Reports
-      uses: actions/upload-artifact@master
-      if: always()
-      with:
-        name: screenshot-test-report
-        retention-days: 30
-        path: |
-          **/build/reports/**
-          **/build/changed-images/**
-          **/build/test-results/**
+      - uses: actions/checkout@v4
+      - name: Start xvfb
+        run: |
+          Xvfb :99 -ac -screen 0 1024x768x16 &
+          export DISPLAY=:99
+          echo "DISPLAY=:99" >> $GITHUB_ENV
+      - name: Report GL/Vulkan
+        run: |
+          set -x
+          echo "DISPLAY=$DISPLAY"
+          glxinfo | grep -E "OpenGL version|OpenGL renderer|OpenGL vendor" || true
+          vulkaninfo --summary || true
+          echo "VK_ICD_FILENAMES=$VK_ICD_FILENAMES"
+          echo "MESA_LOADER_DRIVER_OVERRIDE=$MESA_LOADER_DRIVER_OVERRIDE"
+          echo "GALLIUM_DRIVER=$GALLIUM_DRIVER"
+      - name: Validate the Gradle wrapper
+        uses: gradle/actions/wrapper-validation@v3
+      - name: Test with Gradle Wrapper
+        run: |
+          ./gradlew :jme3-screenshot-test:screenshotTest
+      - name: Upload Test Reports
+        uses: actions/upload-artifact@master
+        if: always()
+        with:
+          name: screenshot-test-report
+          retention-days: 30
+          path: |
+            **/build/reports/**
+            **/build/changed-images/**
+            **/build/test-results/**
   # Build the natives on android
   BuildAndroidNatives:
     name: Build natives for android

.github/workflows/screenshot-test-comment.yml

@@ -21,7 +21,7 @@ jobs:
       contents: read
     steps:
       - name: Wait for GitHub to register the workflow run
-        run: sleep 15
+        run: sleep 120
 
       - name: Wait for Screenshot Tests to complete
         uses: lewagon/[email protected]

jme3-screenshot-tests/src/main/java/org/jmonkeyengine/screenshottests/testframework/App.java

@@ -36,6 +36,8 @@ import com.jme3.app.state.AppState;
 import com.jme3.app.state.VideoRecorderAppState;
 import com.jme3.math.ColorRGBA;
 
+import java.util.function.Consumer;
+
 /**
  * The app used for the tests. AppState(s) are used to inject the actual test code.
  * @author Richard Tingle (aka richtea)
@@ -46,10 +48,17 @@ public class App extends SimpleApplication {
         super(initialStates);
     }
 
+    Consumer<Throwable> onError = (onError) -> {};
+
     @Override
     public void simpleInitApp(){
         getViewPort().setBackgroundColor(ColorRGBA.Black);
         setTimer(new VideoRecorderAppState.IsoTimer(60));
     }
 
+    @Override
+    public void handleError(String errMsg, Throwable t) {
+        super.handleError(errMsg, t);
+        onError.accept(t);
+    }
 }

jme3-screenshot-tests/src/main/java/org/jmonkeyengine/screenshottests/testframework/ExtentReportExtension.java

@@ -50,7 +50,7 @@ import java.util.Optional;
  */
 public class ExtentReportExtension implements BeforeAllCallback, AfterAllCallback, TestWatcher, BeforeTestExecutionCallback{
     private static ExtentReports extent;
-    private static final ThreadLocal<ExtentTest> test = new ThreadLocal<>();
+    private static ExtentTest currentTest;
 
     @Override
     public void beforeAll(ExtensionContext context) {
@@ -62,6 +62,8 @@ public class ExtentReportExtension implements BeforeAllCallback, AfterAllCallbac
             extent = new ExtentReports();
             extent.attachReporter(spark);
         }
+        // Initialize log capture to redirect console output to the report
+        ExtentReportLogCapture.initialize();
     }
 
     @Override
@@ -71,6 +73,9 @@ public class ExtentReportExtension implements BeforeAllCallback, AfterAllCallbac
         * anywhere else I can hook into the lifecycle of the end of all tests to write the report.
         */
         extent.flush();
+
+        // Restore the original System.out
+        ExtentReportLogCapture.restore();
     }
 
     @Override
@@ -96,10 +101,11 @@ public class ExtentReportExtension implements BeforeAllCallback, AfterAllCallbac
     @Override
     public void beforeTestExecution(ExtensionContext context) {
         String testName = context.getDisplayName();
-        test.set(extent.createTest(testName));
+        String className = context.getRequiredTestClass().getSimpleName();
+        currentTest = extent.createTest(className + "." + testName);
     }
 
     public static ExtentTest getCurrentTest() {
-        return test.get();
+        return currentTest;
     }
-}
+}

jme3-screenshot-tests/src/main/java/org/jmonkeyengine/screenshottests/testframework/ExtentReportLogCapture.java

@@ -0,0 +1,117 @@
+/*
+ * Copyright (c) 2025 jMonkeyEngine
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in the
+ *   documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of 'jMonkeyEngine' nor the names of its contributors
+ *   may be used to endorse or promote products derived from this software
+ *   without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.jmonkeyengine.screenshottests.testframework;
+
+import com.aventstack.extentreports.ExtentTest;
+
+import java.io.OutputStream;
+import java.io.PrintStream;
+
+/**
+ * This class captures console logs and adds them to the ExtentReport.
+ * It redirects System.out to both the original console and the ExtentReport.
+ *
+ * @author Richard Tingle (aka richtea)
+ */
+public class ExtentReportLogCapture {
+
+    private static final PrintStream originalOut = System.out;
+    private static final PrintStream originalErr = System.err;
+    private static boolean initialized = false;
+
+    /**
+     * Initializes the log capture system. This should be called once at the start of the test suite.
+     */
+    public static void initialize() {
+        if (!initialized) {
+            // Redirect System.out and System.err
+            System.setOut(new ExtentReportPrintStream(originalOut));
+            System.setErr(new ExtentReportPrintStream(originalErr));
+
+            initialized = true;
+        }
+    }
+
+    /**
+     * Restores the original System.out. This should be called at the end of the test suite.
+     */
+    public static void restore() {
+        if(initialized) {
+            // Restore System.out and System.err
+            System.setOut(originalOut);
+            System.setErr(originalErr);
+            initialized = false;
+        }
+    }
+
+    /**
+     * A custom PrintStream that redirects output to both the original console and the ExtentReport.
+     */
+    private static class ExtentReportPrintStream extends PrintStream {
+        private StringBuilder buffer = new StringBuilder();
+
+        public ExtentReportPrintStream(OutputStream out) {
+            super(out, true);
+        }
+
+        @Override
+        public void write(byte[] buf, int off, int len) {
+            super.write(buf, off, len);
+
+            // Convert the byte array to a string and add to buffer
+            String s = new String(buf, off, len);
+            buffer.append(s);
+
+            // If we have a complete line (ends with newline), process it
+            if (s.endsWith("\n") || s.endsWith("\r\n")) {
+                String line = buffer.toString().trim();
+                if (!line.isEmpty()) {
+                    addToExtentReport(line);
+                }
+                buffer.setLength(0); // Clear the buffer
+            }
+        }
+
+        private void addToExtentReport(String s) {
+            try {
+                ExtentTest currentTest = ExtentReportExtension.getCurrentTest();
+                if (currentTest != null) {
+                    currentTest.info(s);
+                }
+            } catch (Exception e) {
+                // If there's an error adding to the report, just continue
+                // This ensures that console logs are still displayed even if there's an issue with the report
+            }
+        }
+    }
+
+}

jme3-screenshot-tests/src/main/java/org/jmonkeyengine/screenshottests/testframework/TestDriver.java

@@ -57,8 +57,12 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.fail;
@@ -72,6 +76,8 @@ import static org.junit.jupiter.api.Assertions.fail;
  */
 public class TestDriver extends BaseAppState{
 
+    private static final Logger logger = Logger.getLogger(TestDriver.class.getName());
+
     public static final String IMAGES_ARE_DIFFERENT = "Images are different. (If you are running the test locally this is expected, images only reproducible on github CI infrastructure)";
 
     public static final String IMAGES_ARE_DIFFERENT_SIZES = "Images are different sizes.";
@@ -94,7 +100,7 @@ public class TestDriver extends BaseAppState{
 
     ScreenshotNoInputAppState screenshotAppState;
 
-    private final Object waitLock = new Object();
+    private CountDownLatch waitLatch;
 
     private final int tickToTerminateApp;
 
@@ -113,15 +119,19 @@ public class TestDriver extends BaseAppState{
         }
         if(tick >= tickToTerminateApp){
             getApplication().stop(true);
-            synchronized (waitLock) {
-                waitLock.notify(); // Release the wait
-            }
+            waitLatch.countDown();
         }
 
         tick++;
     }
 
-    @Override protected void initialize(Application app){}
+    @Override protected void initialize(Application app){
+        ((App)app).onError = error -> {
+            logger.log(Level.WARNING, "Error in test application", error);
+            waitLatch.countDown();
+        };
+
+    }
 
     @Override protected void cleanup(Application app){}
 
@@ -129,7 +139,6 @@ public class TestDriver extends BaseAppState{
 
     @Override protected void onDisable(){}
 
-
     /**
      * Boots up the application on a separate thread (blocks this thread) and then does the following:
      * - Takes screenshots on the requested frames
@@ -161,16 +170,23 @@ public class TestDriver extends BaseAppState{
         app.setSettings(appSettings);
         app.setShowSettings(false);
 
+        testDriver.waitLatch = new CountDownLatch(1);
         executor.execute(() -> app.start(JmeContext.Type.Display));
 
-        synchronized (testDriver.waitLock) {
-            try {
-                testDriver.waitLock.wait(10000); // Wait for the screenshot to be taken and application to stop
-                Thread.sleep(200); //give time for openGL is fully released before starting a new test (get random JVM crashes without this)
-            } catch (InterruptedException e) {
-                Thread.currentThread().interrupt();
-                throw new RuntimeException(e);
+        int maxWaitTimeMilliseconds = 45000;
+
+        try {
+            boolean exitedProperly = testDriver.waitLatch.await(maxWaitTimeMilliseconds, TimeUnit.MILLISECONDS);
+
+            if(!exitedProperly){
+                logger.warning("Test driver did not exit in " + maxWaitTimeMilliseconds + "ms. Timed out");
+                app.stop(true);
             }
+
+            Thread.sleep(1000); //give time for openGL is fully released before starting a new test (get random JVM crashes without this)
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new RuntimeException(e);
         }
 
         //search the imageTempDir