PPAPI/NaCl: Make related tests run in 1 fixture

ppapi/tests/test_case.cc

Index: ppapi/tests/test_case.cc
diff --git a/ppapi/tests/test_case.cc b/ppapi/tests/test_case.cc
index f6fb485687223161f431378606dcd123fd57db5e..929be18cc411b209b4dab47d66243e46e7dd4109 100644
--- a/ppapi/tests/test_case.cc
+++ b/ppapi/tests/test_case.cc
@@ -4,16 +4,89 @@
 
 #include "ppapi/tests/test_case.h"
 
+#include <string.h>
+
 #include <sstream>
 
+#include "ppapi/cpp/core.h"
+#include "ppapi/cpp/module.h"
 #include "ppapi/tests/pp_thread.h"
 #include "ppapi/tests/test_utils.h"
 #include "ppapi/tests/testing_instance.h"
 
+namespace {
+
+std::string StripPrefixes(const std::string& test_name) {

[bbudge, 2013/02/06 14:51:26]

s/StripPrefixes/StripPrefix

+  const char* const prefixes[] = {
+      "FAILS_", "FLAKY_", "DISABLED_" };
+  for (size_t i = 0; i < sizeof(prefixes)/sizeof(prefixes[0]); ++i)
+    if (test_name.find(prefixes[i]) == 0)
+      return test_name.substr(strlen(prefixes[i]));
+  return test_name;
+}
+
+// Strip the TestCase name off and return the remainder (i.e., everything after
+// '_'). If there is no '_', assume only the TestCase was provided, and return
+// an empty string.
+// For example:
+//   StripTestCase("Foo_TestName");
+// returns
+//   "TestName"
+// while
+//   StripTestCase("Foo);
+// returns
+//   "Foo"

[bbudge, 2013/02/06 14:51:26]

I had trouble understanding this documentation. Maybe replace "Foo" with
"TestCase"? Also, the last line should be "", right?

+std::string StripTestCase(const std::string& full_test_name) {
+  size_t delim = full_test_name.find_first_of('_');
+  if (delim != std::string::npos)
+    return full_test_name.substr(delim+1);
+  // In this case, our "filter" is the empty string; the full test name is the
+  // same as the TestCase name with which we were constructed.
+  // TODO(dmichael): It might be nice to be able to PP_DCHECK against the
+  // TestCase class name, but we'd have to plumb that name to TestCase somehow.
+  return std::string();
+}
+
+// Parse |test_filter|, which is a comma-delimited list of (possibly prefixed)
+// test names and insert the un-prefixed names in to |remaining_tests|, with
+// the bool indicating whether the test should be run.
+void ParseTestFilter(std::string test_filter,
+                     std::map<std::string, bool>* remaining_tests) {
+  // We can't use base/string_util.h::Tokenize in ppapi. Instead, we'll

[bbudge, 2013/02/06 14:51:26]

Did this comment get cut off?

+  while (!test_filter.empty()) {
+    size_t end_of_first_test_name = test_filter.find_first_of(",");
+    // Note |end_of_first_test_name| could be npos, which means "take all
+    // characters" to substr, so this is good for lists of 1 also.
+
+    // |current_test| includes prefixes, if any, like DISABLED_Foo_TestBar
+    std::string current_test(test_filter.substr(0, end_of_first_test_name));
+
+    // Advance past the first item to the next.
+    test_filter.erase(0, end_of_first_test_name + 1);
+
+    std::string stripped_test_name(StripPrefixes(current_test));
+    // We'll strip the case for use as a key, because the test name

[bbudge, 2013/02/06 14:51:26]

This is a little confusing to me. Maybe say "Strip off the test case and use the
test name as the key ..."

+    // MatchesFilter wants to use to look up the test doesn't have the TestCase
+    // name.
+    std::string test_name_without_case(StripTestCase(stripped_test_name));
+
+    // If the test wasn't prefixed, it should be run.
+    bool should_run_test = (current_test == stripped_test_name);
+    PP_DCHECK(remaining_tests->count(test_name_without_case) == 0);
+    remaining_tests->insert(
+        std::make_pair(test_name_without_case, should_run_test));
+  }
+  // There may be a trailing comma; ignore empty strings.
+  remaining_tests->erase(std::string());
+}
+
+}  // namespace
+
 TestCase::TestCase(TestingInstance* instance)
     : instance_(instance),
       testing_interface_(NULL),
-      callback_type_(PP_REQUIRED) {
+      callback_type_(PP_REQUIRED),
+      have_populated_remaining_tests_(false) {
   // Get the testing_interface_ if it is available, so that we can do Resource
   // and Var checks on shutdown (see CheckResourcesAndVars). If it is not
   // available, testing_interface_ will be NULL. Some tests do not require it.
@@ -105,7 +178,29 @@ bool TestCase::EnsureRunningOverHTTP() {
 
 bool TestCase::MatchesFilter(const std::string& test_name,

[bbudge, 2013/02/06 14:51:26]

nit: Maybe this method should be named "ShouldRunTest" now?

                              const std::string& filter) {
-  return filter.empty() || (test_name == filter);
+  // If only the TestCase is listed, we're running all the tests in RunTests.
+  if (StripTestCase(filter) == std::string())
+    return true;
+
+  // Lazily initialize our "remaining_tests_" map.
+  if (!have_populated_remaining_tests_) {
+    ParseTestFilter(filter, &remaining_tests_);
+    have_populated_remaining_tests_ = true;
+  }
+  std::map<std::string, bool>::iterator iter = remaining_tests_.find(test_name);
+  if (iter == remaining_tests_.end()) {
+    // The test name wasn't listed in the filter. Don't run it, but store it
+    // so TestingInstance::ExecuteTests can report an error later.
+    skipped_tests_.insert(test_name);
+    return false;
+  }
+  bool should_run_test = iter->second;
+  remaining_tests_.erase(iter);
+  return should_run_test;
+}
+
+PP_TimeTicks TestCase::NowInTimeTicks() {
+  return pp::Module::Get()->core()->GetTimeTicks();
 }
 
 std::string TestCase::CheckResourcesAndVars(std::string errors) {