summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Nolan Scobie <nscobie@google.com> 2024-06-27 17:04:12 -0400
committer Nolan Scobie <nscobie@google.com> 2024-07-01 17:15:41 -0400
commitbe6f9e80aa2598305c0d2a2bb380bcb320d0a7f2 (patch)
tree941c601c8ab1ac8b7e31ca1a597e0df321a537cb
parent9c879e83fd29494822cc8bc4e7a4692145512556 (diff)
Update RenderEngineTest's buffer dumping file paths
Writing to `/data/file.ppm` was failing for me, even with `adb shell setenforce 0`. Changing this to `/data/local/tmp/file.ppm` let the write succeed, event without `adb shell setenforce 0`. I chose `/data/local/tmp/` based on context clues from other tests, but am happy with any directory, as long as we have write permission. After feedback I've also changed the code to create the target directory if it doesn't exist, which allows us to use `/data/local/tmp/RenderEngineTest` and remove the `texture_out_` file name prefix. Note: both `/data/` and `/data/local/tmp/` have the same permissions of 771, but they're owned by different users, `system` and `shell` respectively. I also tweaked the file name since tests parameterized by RenderEngine type were appending e.g. `/0`. Bug: N/A Test: manual Flag: EXEMPT test only Change-Id: I3017b870ecb4d61cd90206128774d45b003e3cf0
-rw-r--r--libs/renderengine/tests/RenderEngineTest.cpp51
1 files changed, 42 insertions, 9 deletions
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index a8a98236e2..3c59fb30b1 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -34,9 +34,12 @@
#include <ui/ColorSpace.h>
#include <ui/PixelFormat.h>
+#include <algorithm>
#include <chrono>
#include <condition_variable>
+#include <filesystem>
#include <fstream>
+#include <system_error>
#include "../skia/SkiaGLRenderEngine.h"
#include "../skia/SkiaVkRenderEngine.h"
@@ -259,22 +262,51 @@ public:
~RenderEngineTest() {
if (WRITE_BUFFER_TO_FILE_ON_FAILURE && ::testing::Test::HasFailure()) {
- writeBufferToFile("/data/texture_out_");
+ writeBufferToFile("/data/local/tmp/RenderEngineTest/");
}
const ::testing::TestInfo* const test_info =
::testing::UnitTest::GetInstance()->current_test_info();
ALOGI("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name());
}
- void writeBufferToFile(const char* basename) {
- std::string filename(basename);
- filename.append(::testing::UnitTest::GetInstance()->current_test_info()->name());
- filename.append(".ppm");
- std::ofstream file(filename.c_str(), std::ios::binary);
+ // If called during e.g.
+ // `PerRenderEngineType/RenderEngineTest#drawLayers_fillBufferCheckersRotate90_colorSource/0`
+ // with a directory of `/data/local/tmp/RenderEngineTest`, then mBuffer will be dumped to
+ // `/data/local/tmp/RenderEngineTest/drawLayers_fillBufferCheckersRotate90_colorSource-0.ppm`
+ //
+ // Note: if `directory` does not exist, then its full path will be recursively created with 777
+ // permissions. If `directory` already exists but does not grant the executing user write
+ // permissions, then saving the buffer will fail.
+ //
+ // Since this is test-only code, no security considerations are made.
+ void writeBufferToFile(const filesystem::path& directory) {
+ const auto currentTestInfo = ::testing::UnitTest::GetInstance()->current_test_info();
+ LOG_ALWAYS_FATAL_IF(!currentTestInfo,
+ "writeBufferToFile must be called during execution of a test");
+
+ std::string fileName(currentTestInfo->name());
+ // Test names may include the RenderEngine variant separated by '/', which would separate
+ // the file name into a subdirectory if not corrected.
+ std::replace(fileName.begin(), fileName.end(), '/', '-');
+ fileName.append(".ppm");
+
+ std::error_code err;
+ filesystem::create_directories(directory, err);
+ if (err.value()) {
+ ALOGE("Unable to create directory %s for writing %s (%d: %s)", directory.c_str(),
+ fileName.c_str(), err.value(), err.message().c_str());
+ return;
+ }
+
+ // Append operator ("/") ensures exactly one "/" directly before the argument.
+ const filesystem::path filePath = directory / fileName;
+ std::ofstream file(filePath.c_str(), std::ios::binary);
if (!file.is_open()) {
- ALOGE("Unable to open file: %s", filename.c_str());
- ALOGE("You may need to do: \"adb shell setenforce 0\" to enable "
- "surfaceflinger to write debug images");
+ ALOGE("Unable to open file: %s", filePath.c_str());
+ ALOGE("You may need to do: \"adb shell setenforce 0\" to enable surfaceflinger to "
+ "write debug images, or the %s directory might not give the executing user write "
+ "permission",
+ directory.c_str());
return;
}
@@ -304,6 +336,7 @@ public:
}
}
file.write(reinterpret_cast<char*>(outBuffer.data()), outBuffer.size());
+ ALOGI("Image of incorrect output written to %s", filePath.c_str());
mBuffer->getBuffer()->unlock();
}