Fix bug in setting directories for tests.
make_canonical_dir_path only worked if the provided directory
did not end with a slash. Remove this function, and call
SkPathJoin instead. Update the documentation to acknowledge
that this is an acceptable use of SkPathJoin, and update its
test.
R=epoger@google.com
Review URL: https://codereview.chromium.org/16098011
git-svn-id: http://skia.googlecode.com/svn/trunk@9458 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/include/core/SkOSFile.h b/include/core/SkOSFile.h
index f8ce06b..b75fe6c 100644
--- a/include/core/SkOSFile.h
+++ b/include/core/SkOSFile.h
@@ -138,7 +138,9 @@
public:
/**
* Assembles rootPath and relativePath into a single path, like this:
- * rootPath/relativePath
+ * rootPath/relativePath.
+ * It is okay to call with a NULL rootPath and/or relativePath. A path
+ * separator will still be inserted.
*
* Uses SkPATH_SEPARATOR, to work on all platforms.
*/
diff --git a/tests/OSPathTest.cpp b/tests/OSPathTest.cpp
index 96ff8a7..5ae32eb 100644
--- a/tests/OSPathTest.cpp
+++ b/tests/OSPathTest.cpp
@@ -59,6 +59,12 @@
dir.appendUnichar(SkPATH_SEPARATOR);
test_dir_with_file(reporter, dir, filename);
+ // Test using no filename.
+ test_dir_with_file(reporter, dir, SkString());
+
+ // Testing using no directory.
+ test_dir_with_file(reporter, SkString(), filename);
+
// Test with a sub directory.
dir.append("subDir");
test_dir_with_file(reporter, dir, filename);
@@ -71,6 +77,11 @@
// Basename of NULL is an empty string.
SkString empty = SkOSPath::SkBasename(NULL);
REPORTER_ASSERT(reporter, empty.size() == 0);
+
+ // Test that NULL can be used for the directory and filename.
+ SkString emptyPath = SkOSPath::SkPathJoin(NULL, NULL);
+ REPORTER_ASSERT(reporter, emptyPath.size() == 1);
+ REPORTER_ASSERT(reporter, emptyPath.contains(SkPATH_SEPARATOR));
}
#include "TestClassDef.h"
diff --git a/tests/StreamTest.cpp b/tests/StreamTest.cpp
index 8f38f46..00c079a 100644
--- a/tests/StreamTest.cpp
+++ b/tests/StreamTest.cpp
@@ -35,8 +35,7 @@
}
static void test_filestreams(skiatest::Reporter* reporter, const char* tmpDir) {
- SkString path;
- path.printf("%s%s", tmpDir, "wstream_test");
+ SkString path = SkOSPath::SkPathJoin(tmpDir, "wstream_test");
const char s[] = "abcdefghijklmnopqrstuvwxyz";
@@ -98,8 +97,9 @@
}
delete[] dst;
- if (!skiatest::Test::GetTmpDir().isEmpty()) {
- test_filestreams(reporter, skiatest::Test::GetTmpDir().c_str());
+ SkString tmpDir = skiatest::Test::GetTmpDir();
+ if (!tmpDir.isEmpty()) {
+ test_filestreams(reporter, tmpDir.c_str());
}
}
diff --git a/tests/Test.h b/tests/Test.h
index dfa12a6..8cb23c1 100644
--- a/tests/Test.h
+++ b/tests/Test.h
@@ -75,9 +75,9 @@
bool passed() const { return fPassed; }
SkMSec elapsedMs() const { return fElapsed; }
- static const SkString& GetTmpDir();
+ static SkString GetTmpDir();
- static const SkString& GetResourcePath();
+ static SkString GetResourcePath();
virtual bool isThreadsafe() const { return true; }
diff --git a/tests/skia_test.cpp b/tests/skia_test.cpp
index 238252d..8cae656 100644
--- a/tests/skia_test.cpp
+++ b/tests/skia_test.cpp
@@ -112,34 +112,6 @@
bool fAllowThreaded;
};
-static const char* make_canonical_dir_path(const char* path, SkString* storage) {
- if (path) {
- // clean it up so it always has a trailing searator
- size_t len = strlen(path);
- if (0 == len) {
- path = NULL;
- } else if (SkPATH_SEPARATOR != path[len - 1]) {
- // resize to len + 1, to make room for searator
- storage->set(path, len + 1);
- storage->writable_str()[len] = SkPATH_SEPARATOR;
- path = storage->c_str();
- }
- }
- return path;
-}
-
-static SkString gTmpDir;
-
-const SkString& Test::GetTmpDir() {
- return gTmpDir;
-}
-
-static SkString gResourcePath;
-
-const SkString& Test::GetResourcePath() {
- return gResourcePath;
-}
-
DEFINE_string2(match, m, NULL, "[~][^]substring[$] [...] of test name to run.\n" \
"Multiple matches may be separated by spaces.\n" \
"~ causes a matching test to always be skipped\n" \
@@ -156,6 +128,16 @@
DEFINE_int32(threads, SkThreadPool::kThreadPerCore,
"Run threadsafe tests on a threadpool with this many threads.");
+SkString Test::GetTmpDir() {
+ const char* tmpDir = FLAGS_tmpDir.isEmpty() ? NULL : FLAGS_tmpDir[0];
+ return SkString(tmpDir);
+}
+
+SkString Test::GetResourcePath() {
+ const char* resourcePath = FLAGS_resourcePath.isEmpty() ? NULL : FLAGS_resourcePath[0];
+ return SkString(resourcePath);
+}
+
// Deletes self when run.
class SkTestRunnable : public SkRunnable {
public:
@@ -218,13 +200,6 @@
SkCommandLineFlags::SetUsage("");
SkCommandLineFlags::Parse(argc, argv);
- if (!FLAGS_tmpDir.isEmpty()) {
- make_canonical_dir_path(FLAGS_tmpDir[0], &gTmpDir);
- }
- if (!FLAGS_resourcePath.isEmpty()) {
- make_canonical_dir_path(FLAGS_resourcePath[0], &gResourcePath);
- }
-
#if SK_ENABLE_INST_COUNT
gPrintInstCount = true;
#endif
@@ -239,11 +214,13 @@
header.appendf(" %s", FLAGS_match[index]);
}
}
- if (!gTmpDir.isEmpty()) {
- header.appendf(" --tmpDir %s", gTmpDir.c_str());
+ SkString tmpDir = Test::GetTmpDir();
+ if (!tmpDir.isEmpty()) {
+ header.appendf(" --tmpDir %s", tmpDir.c_str());
}
- if (!gResourcePath.isEmpty()) {
- header.appendf(" --resourcePath %s", gResourcePath.c_str());
+ SkString resourcePath = Test::GetResourcePath();
+ if (!resourcePath.isEmpty()) {
+ header.appendf(" --resourcePath %s", resourcePath.c_str());
}
#ifdef SK_DEBUG
header.append(" SK_DEBUG");