From 0440c8cb821f6216d61f2c7ebfb1e61abf8988be Mon Sep 17 00:00:00 2001 From: Dharmendra Singh Date: Mon, 16 Dec 2019 10:49:09 +0530 Subject: [PATCH] Write screenshots to file and return the file path instead of returning screenshots bytes. getgauge/gauge#1476 --- .../java/com/thoughtworks/gauge/Gauge.java | 10 +++--- .../gauge/ScreenshotCollector.java | 13 +++---- .../execution/AbstractExecutionStage.java | 6 ++-- .../gauge/execution/MethodExecutor.java | 6 ++-- .../gauge/screenshot/ScreenshotFactory.java | 35 +++++++++++------- .../gauge/ScreenshotCollectorTest.java | 17 +++++---- .../execution/AbstractExecutionStageTest.java | 36 ++++++++----------- 7 files changed, 58 insertions(+), 65 deletions(-) diff --git a/src/main/java/com/thoughtworks/gauge/Gauge.java b/src/main/java/com/thoughtworks/gauge/Gauge.java index 5fa54ecc..5f353220 100644 --- a/src/main/java/com/thoughtworks/gauge/Gauge.java +++ b/src/main/java/com/thoughtworks/gauge/Gauge.java @@ -29,9 +29,9 @@ protected List initialValue() { } }; private static ClassInstanceManager instanceManager; - private static ThreadLocal> screenshots = new InheritableThreadLocal>() { + private static ThreadLocal> screenshots = new InheritableThreadLocal>() { @Override - protected List initialValue() { + protected List initialValue() { return new ArrayList<>(); } }; @@ -61,11 +61,11 @@ static List getMessages() { } public static void captureScreenshot() { - byte[] screenshotBytes = new ScreenshotFactory(instanceManager).getScreenshotBytes(); - getScreenshots().add(screenshotBytes); + String screenshotFileName = new ScreenshotFactory(instanceManager).getScreenshotBytes(); + getScreenshots().add(screenshotFileName); } - static List getScreenshots() { + static List getScreenshots() { return screenshots.get(); } } diff --git a/src/main/java/com/thoughtworks/gauge/ScreenshotCollector.java b/src/main/java/com/thoughtworks/gauge/ScreenshotCollector.java index fdbfacff..7adee7f0 100644 --- a/src/main/java/com/thoughtworks/gauge/ScreenshotCollector.java +++ b/src/main/java/com/thoughtworks/gauge/ScreenshotCollector.java @@ -15,7 +15,6 @@ package com.thoughtworks.gauge; -import com.google.protobuf.ByteString; import gauge.messages.Spec; import java.util.ArrayList; @@ -25,20 +24,18 @@ public class ScreenshotCollector { public Spec.ProtoExecutionResult addPendingScreenshotTo(Spec.ProtoExecutionResult result) { - List screenshots = getAllPendingScreenshots(); + ArrayList screenshots = getAllPendingScreenshots(); return addPendingScreenshot(result, screenshots); } - Spec.ProtoExecutionResult addPendingScreenshot(Spec.ProtoExecutionResult result, List screenshots) { + Spec.ProtoExecutionResult addPendingScreenshot(Spec.ProtoExecutionResult result, List screenshotsFileName) { Spec.ProtoExecutionResult.Builder builder = Spec.ProtoExecutionResult.newBuilder(result); - for (byte[] screenshot : screenshots) { - builder.addScreenshots(ByteString.copyFrom(screenshot)); - } + builder.addAllScreenshotFiles(screenshotsFileName); return builder.build(); } - private List getAllPendingScreenshots() { - List pendingScreenshots = new ArrayList<>(getScreenshots()); + private ArrayList getAllPendingScreenshots() { + ArrayList pendingScreenshots = new ArrayList<>(getScreenshots()); clear(); return pendingScreenshots; } diff --git a/src/main/java/com/thoughtworks/gauge/execution/AbstractExecutionStage.java b/src/main/java/com/thoughtworks/gauge/execution/AbstractExecutionStage.java index d56ae598..fc358761 100644 --- a/src/main/java/com/thoughtworks/gauge/execution/AbstractExecutionStage.java +++ b/src/main/java/com/thoughtworks/gauge/execution/AbstractExecutionStage.java @@ -37,15 +37,13 @@ protected Spec.ProtoExecutionResult mergeExecResults(Spec.ProtoExecutionResult p if (previousStageResult.getFailed()) { builder.setErrorMessage(previousStageResult.getErrorMessage()); builder.setErrorType(previousStageResult.getErrorType()); - builder.setScreenShot(previousStageResult.getScreenShot()); - builder.setFailureScreenshot(previousStageResult.getFailureScreenshot()); + builder.setFailureScreenshotFile(previousStageResult.getFailureScreenshotFile()); builder.setStackTrace(previousStageResult.getStackTrace()); builder.setRecoverableError(previousStageResult.getRecoverableError()); } else if (execResult.getFailed()) { builder.setErrorType(execResult.getErrorType()); builder.setErrorMessage(execResult.getErrorMessage()); - builder.setScreenShot(execResult.getScreenShot()); - builder.setFailureScreenshot(execResult.getFailureScreenshot()); + builder.setFailureScreenshotFile(execResult.getFailureScreenshotFile()); builder.setStackTrace(execResult.getStackTrace()); builder.setRecoverableError(execResult.getRecoverableError()); } diff --git a/src/main/java/com/thoughtworks/gauge/execution/MethodExecutor.java b/src/main/java/com/thoughtworks/gauge/execution/MethodExecutor.java index 0a88dc59..f2ff0a25 100644 --- a/src/main/java/com/thoughtworks/gauge/execution/MethodExecutor.java +++ b/src/main/java/com/thoughtworks/gauge/execution/MethodExecutor.java @@ -15,7 +15,6 @@ package com.thoughtworks.gauge.execution; -import com.google.protobuf.ByteString; import com.thoughtworks.gauge.ClassInstanceManager; import com.thoughtworks.gauge.ContinueOnFailure; import com.thoughtworks.gauge.screenshot.ScreenshotFactory; @@ -51,9 +50,8 @@ public Spec.ProtoExecutionResult execute(Method method, Object... args) { private Spec.ProtoExecutionResult createFailureExecResult(long execTime, Throwable e, boolean recoverable, Class[] continuableExceptions) { Spec.ProtoExecutionResult.Builder builder = Spec.ProtoExecutionResult.newBuilder().setFailed(true); - ByteString screenshotBytes = ByteString.copyFrom(new ScreenshotFactory(instanceManager).getScreenshotBytes()); - builder.setScreenShot(screenshotBytes); - builder.setFailureScreenshot(screenshotBytes); + String screenshotFileName = new ScreenshotFactory(instanceManager).getScreenshotBytes(); + builder.setFailureScreenshotFile(screenshotFileName); if (e.getCause() != null) { builder.setRecoverableError(false); for (Class c : continuableExceptions) { diff --git a/src/main/java/com/thoughtworks/gauge/screenshot/ScreenshotFactory.java b/src/main/java/com/thoughtworks/gauge/screenshot/ScreenshotFactory.java index 6fd83f9d..76728c6b 100644 --- a/src/main/java/com/thoughtworks/gauge/screenshot/ScreenshotFactory.java +++ b/src/main/java/com/thoughtworks/gauge/screenshot/ScreenshotFactory.java @@ -25,7 +25,11 @@ import java.awt.Rectangle; import java.awt.Robot; import java.awt.image.BufferedImage; -import java.io.ByteArrayOutputStream; +import java.io.ByteArrayInputStream; +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.UUID; /** * Used to take screenshots on failure. @@ -44,22 +48,23 @@ static void setCustomScreenshotGrabber(Class customScreenshotGrabber = customScreenGrabber; } - public byte[] getScreenshotBytes() { + public String getScreenshotBytes() { if (shouldTakeScreenshot()) { return takeScreenshot(); } - return new byte[0]; + return ""; } - private byte[] takeScreenshot() { + private String takeScreenshot() { if (customScreenshotGrabber != null) { try { ICustomScreenshotGrabber customScreenGrabberInstance = (ICustomScreenshotGrabber) manager.get(customScreenshotGrabber); byte[] bytes = customScreenGrabberInstance.takeScreenshot(); - if (bytes == null) { - bytes = new byte[0]; - } - return bytes; + File file = generateUniqueScreenshotFile(); + ByteArrayInputStream byteArrayInputStream = new ByteArrayInputStream(bytes); + BufferedImage bufferedImage = ImageIO.read(byteArrayInputStream); + ImageIO.write(bufferedImage, IMAGE_EXTENSION, file); + return file.getName(); } catch (Exception e) { Logger.error(String.format("Failed to take Custom screenshot: %s : %s", customScreenshotGrabber.getCanonicalName(), e.getMessage())); Logger.warning("Capturing regular screenshot.."); @@ -68,8 +73,13 @@ private byte[] takeScreenshot() { return captureScreen(); } - private byte[] captureScreen() { - ByteArrayOutputStream imageBytes = new ByteArrayOutputStream(); + private File generateUniqueScreenshotFile() { + Path path = Paths.get(System.getenv("screenshots_dir"), String.format("screenshot-%s.png", UUID.randomUUID().toString())); + return new File(path.toAbsolutePath().toString()); + } + + private String captureScreen() { + File file = generateUniqueScreenshotFile(); if (shouldTakeScreenshot()) { try { // Union together all screen devices for 1 large screenshot @@ -78,13 +88,12 @@ private byte[] captureScreen() { screenRect = screenRect.union(gd.getDefaultConfiguration().getBounds()); } BufferedImage image = new Robot().createScreenCapture(screenRect); - ImageIO.write(image, IMAGE_EXTENSION, imageBytes); + ImageIO.write(image, IMAGE_EXTENSION, file); } catch (Throwable e) { Logger.error("Failed to take regular screenshot: " + e.getMessage()); - return new byte[0]; } } - return imageBytes.toByteArray(); + return file.getName(); } private boolean shouldTakeScreenshot() { diff --git a/src/test/java/com/thoughtworks/gauge/ScreenshotCollectorTest.java b/src/test/java/com/thoughtworks/gauge/ScreenshotCollectorTest.java index 18995316..c2706823 100644 --- a/src/test/java/com/thoughtworks/gauge/ScreenshotCollectorTest.java +++ b/src/test/java/com/thoughtworks/gauge/ScreenshotCollectorTest.java @@ -15,7 +15,6 @@ package com.thoughtworks.gauge; -import com.google.protobuf.ByteString; import gauge.messages.Spec; import junit.framework.TestCase; @@ -25,15 +24,15 @@ public class ScreenshotCollectorTest extends TestCase { public void testAddingScreenshotsToProtoResult() { Spec.ProtoExecutionResult executionResult = emptyExecResult(); - byte a = Byte.valueOf("1"); - byte b = Byte.valueOf("2"); - byte[] bytes = {a,b}; - List screenshots = new ArrayList<>(); - screenshots.add(bytes); + String a = "1"; + String b = "2"; + List screenshots = new ArrayList<>(); + screenshots.add(a); + screenshots.add(b); Spec.ProtoExecutionResult protoExecutionResult = new ScreenshotCollector().addPendingScreenshot(executionResult, screenshots); - List actualScreenshotList = protoExecutionResult.getScreenshotsList(); - for (byte[] screenshot : screenshots) { - assertTrue(actualScreenshotList.contains(ByteString.copyFrom(screenshot))); + List actualScreenshotList = protoExecutionResult.getScreenshotFilesList(); + for (String screenshot : screenshots) { + assertTrue(actualScreenshotList.contains(screenshot)); } } diff --git a/src/test/java/com/thoughtworks/gauge/execution/AbstractExecutionStageTest.java b/src/test/java/com/thoughtworks/gauge/execution/AbstractExecutionStageTest.java index 98c3ad2e..524c93de 100644 --- a/src/test/java/com/thoughtworks/gauge/execution/AbstractExecutionStageTest.java +++ b/src/test/java/com/thoughtworks/gauge/execution/AbstractExecutionStageTest.java @@ -15,7 +15,6 @@ package com.thoughtworks.gauge.execution; -import com.google.protobuf.ByteString; import gauge.messages.Spec; import junit.framework.TestCase; @@ -30,17 +29,14 @@ public void testMergingSimpleResultsBothPassing() throws Exception { } public void testMergingResultsPreviousFailing() throws Exception { - - Byte b = Byte.valueOf("1"); - byte[] bytes = {b}; - ByteString screenShot = ByteString.copyFrom(bytes); + String screenShot = "1"; Spec.ProtoExecutionResult previous = Spec.ProtoExecutionResult.newBuilder().setFailed(true). setExecutionTime(100). setRecoverableError(false). setErrorMessage("Previous failed"). setStackTrace("Previous stacktrace"). - setFailureScreenshot(screenShot).build(); + setFailureScreenshotFile(screenShot).build(); Spec.ProtoExecutionResult current = Spec.ProtoExecutionResult.newBuilder().setFailed(false).setExecutionTime(1100).build(); Spec.ProtoExecutionResult result = new TestExecutionStage().mergeExecResults(previous, current); @@ -49,13 +45,11 @@ public void testMergingResultsPreviousFailing() throws Exception { assertEquals("Previous failed", result.getErrorMessage()); assertEquals("Previous stacktrace", result.getStackTrace()); assertFalse(result.getRecoverableError()); - assertEquals(screenShot, result.getFailureScreenshot()); + assertEquals(screenShot, result.getFailureScreenshotFile()); } public void testMergingResultsCurrentFailing() throws Exception { - Byte b = Byte.valueOf("2"); - byte[] bytes = {b}; - ByteString screenShot = ByteString.copyFrom(bytes); + String screenShot = "2"; Spec.ProtoExecutionResult previous = Spec.ProtoExecutionResult.newBuilder().setFailed(false).setExecutionTime(100).build(); Spec.ProtoExecutionResult current = Spec.ProtoExecutionResult.newBuilder().setFailed(true). @@ -63,7 +57,7 @@ public void testMergingResultsCurrentFailing() throws Exception { setRecoverableError(false). setErrorMessage("current failed"). setStackTrace("current stacktrace"). - setFailureScreenshot(screenShot).build(); + setFailureScreenshotFile(screenShot).build(); Spec.ProtoExecutionResult result = new TestExecutionStage().mergeExecResults(previous, current); assertTrue(result.getFailed()); @@ -71,27 +65,25 @@ public void testMergingResultsCurrentFailing() throws Exception { assertEquals("current failed", result.getErrorMessage()); assertEquals("current stacktrace", result.getStackTrace()); assertFalse(result.getRecoverableError()); - assertEquals(screenShot, result.getFailureScreenshot()); + assertEquals(screenShot, result.getFailureScreenshotFile()); } public void testMergingResultsBothFailing() throws Exception { - Byte b = Byte.valueOf("2"); - byte[] bytes = {b}; - ByteString screenShotPrevious = ByteString.copyFrom(bytes); - ByteString screenShotCurrent = ByteString.copyFromUtf8("hello"); + String screenShotPrevious = "2"; + String screenShotCurrent = "hello"; Spec.ProtoExecutionResult previous = Spec.ProtoExecutionResult.newBuilder().setFailed(true). setExecutionTime(1001). setRecoverableError(true). setErrorMessage("previous failed"). setStackTrace("previous stacktrace"). - setFailureScreenshot(screenShotPrevious).build(); + setFailureScreenshotFile(screenShotPrevious).build(); Spec.ProtoExecutionResult current = Spec.ProtoExecutionResult.newBuilder().setFailed(true). setExecutionTime(1002). setRecoverableError(false). setErrorMessage("current failed"). setStackTrace("current stacktrace"). - setFailureScreenshot(screenShotCurrent).build(); + setFailureScreenshotFile(screenShotCurrent).build(); Spec.ProtoExecutionResult result = new TestExecutionStage().mergeExecResults(previous, current); assertTrue(result.getFailed()); @@ -99,11 +91,11 @@ public void testMergingResultsBothFailing() throws Exception { assertEquals("previous failed", result.getErrorMessage()); assertEquals("previous stacktrace", result.getStackTrace()); assertFalse(result.getRecoverableError()); - assertEquals(screenShotPrevious, result.getFailureScreenshot()); + assertEquals(screenShotPrevious, result.getFailureScreenshotFile()); } public void testMergingResultsCurrentFailingAndIsRecoverable() throws Exception { - ByteString screenShotCurrent = ByteString.copyFromUtf8("hello"); + String screenShotCurrent = "screenShotCurrent.png"; Spec.ProtoExecutionResult previous = Spec.ProtoExecutionResult.newBuilder().setFailed(false).setExecutionTime(1001).build(); Spec.ProtoExecutionResult current = Spec.ProtoExecutionResult.newBuilder().setFailed(true). @@ -111,7 +103,7 @@ public void testMergingResultsCurrentFailingAndIsRecoverable() throws Exception setRecoverableError(true). setErrorMessage("current failed"). setStackTrace("current stacktrace"). - setFailureScreenshot(screenShotCurrent).build(); + setFailureScreenshotFile(screenShotCurrent).build(); Spec.ProtoExecutionResult result = new TestExecutionStage().mergeExecResults(previous, current); assertTrue(result.getFailed()); @@ -119,7 +111,7 @@ public void testMergingResultsCurrentFailingAndIsRecoverable() throws Exception assertEquals("current failed", result.getErrorMessage()); assertEquals("current stacktrace", result.getStackTrace()); assertTrue(result.getRecoverableError()); - assertEquals(screenShotCurrent, result.getFailureScreenshot()); + assertEquals(screenShotCurrent, result.getFailureScreenshotFile()); } private class TestExecutionStage extends AbstractExecutionStage {