Log base::Features in Launch.FlagsAtStartup

chrome/browser/about_flags_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/about_flags.h"
 
 #include <stddef.h>
 
 #include <map>
 #include <set>
 #include <string>
 
 #include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/format_macros.h"
 #include "base/path_service.h"
+#include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "components/flags_ui/feature_entry.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/libxml/chromium/libxml_utils.h"
 
 namespace about_flags {
@@ skipping 135 matching lines @@
 std::string FilePathStringTypeToString(const base::FilePath::StringType& path) {
 #if defined(OS_WIN)
   return base::UTF16ToUTF8(path);
 #else
   return path;
 #endif
 }
 
 // Get all associated switches corresponding to defined about_flags.cc entries.
 // Does not include information about FEATURE_VALUE or
-// FEATURE_WITH_VARIATIOSN_VALUE entries.
+// FEATURE_WITH_VARIATIONS_VALUE entries.
 std::set<std::string> GetAllSwitchesForTesting() {
   std::set<std::string> result;
 
   size_t num_entries = 0;
   const flags_ui::FeatureEntry* entries =
       testing::GetFeatureEntries(&num_entries);
 
   for (size_t i = 0; i < num_entries; ++i) {
     const flags_ui::FeatureEntry& entry = entries[i];
     switch (entry.type) {
@@ skipping 11 matching lines @@
         result.insert(entry.disable_command_line_switch);
         break;
       case flags_ui::FeatureEntry::FEATURE_VALUE:
       case flags_ui::FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
         break;
     }
   }
   return result;
 }
 
+// Get all associated features corresponding to defined about_flags.cc entries.
+// Does not include information about FEATURE_WITH_VARIATIONS_VALUE entries.
+std::set<std::string> GetAllFeaturesForTesting() {
+  std::set<std::string> result;
+
+  size_t num_entries = 0;
+  const flags_ui::FeatureEntry* entries =
+      testing::GetFeatureEntries(&num_entries);
+
+  for (size_t i = 0; i < num_entries; ++i) {
+    const flags_ui::FeatureEntry& entry = entries[i];
+
+    std::string enabled_feature;
+    std::string disabled_feature;
+    switch (entry.type) {
+      case flags_ui::FeatureEntry::FEATURE_VALUE:
+        enabled_feature.append(entry.feature->name);
+        enabled_feature.append(":enabled");
+        disabled_feature.append(entry.feature->name);
+        disabled_feature.append(":disabled");

[Alexei Svitkine (slow), 2016/09/16 15:52:39]

Just inline all of this into the .insert() call and remove the extra local
variables.

i.e.

result.insert(std::string(entry.feature->name) + ":enabled);

[lawrencewu, 2016/09/16 17:12:57]

Done.

+        result.insert(enabled_feature);
+        result.insert(disabled_feature);
+        break;
+      case flags_ui::FeatureEntry::SINGLE_VALUE:
+      case flags_ui::FeatureEntry::SINGLE_DISABLE_VALUE:
+      case flags_ui::FeatureEntry::MULTI_VALUE:
+      case flags_ui::FeatureEntry::ENABLE_DISABLE_VALUE:
+      case flags_ui::FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
+        break;
+    }
+  }
+  return result;
+}
+
 }  // anonymous namespace
 
 // Makes sure there are no separators in any of the entry names.
 TEST(AboutFlagsTest, NoSeparators) {
   size_t count;
   const flags_ui::FeatureEntry* entries = testing::GetFeatureEntries(&count);
   for (size_t i = 0; i < count; ++i) {
     std::string name = entries[i].internal_name;
     EXPECT_EQ(std::string::npos, name.find(flags_ui::testing::kMultiSeparator))
         << i;
@@ skipping 63 matching lines @@
         << "histograms.xml enum LoginCustomFlags "
            "entry '" << entry.second << "' has incorrect value=" << entry.first
         << ", but " << uma_id << " is expected. Consider changing entry to:\n"
         << "  " << GetHistogramEnumEntryText(entry.second, uma_id);
     SetSwitchToHistogramIdMapping(entry.second, entry.first,
                                   &histograms_xml_switches_ids);
   }
 
   // Check that all flags in about_flags.cc have entries in login_custom_flags.
   std::set<std::string> all_switches = GetAllSwitchesForTesting();
-  for (const std::string& flag : all_switches) {
+  std::set<std::string> all_features = GetAllFeaturesForTesting();
+  std::set<std::string> all_flags = base::STLSetUnion<std::set<std::string>>(
+      all_switches,
+      all_features);

[Alexei Svitkine (slow), 2016/09/16 15:52:39]

Given the test just wants this final merged list and nothing else uses
GetAllSwitchesForTesting() - can you just combine the two ForTesting() functions
into one, so it's the only thing that's exposed?

i.e. from the original code just add the extra functionality into the existing
ForTesting() function and maybe rename it (e.g.
GetAllSwitchesAndFeaturesForTesting).

[lawrencewu, 2016/09/16 17:12:57]

Done.

+  for (const std::string& flag : all_flags) {
     // Skip empty placeholders.
     if (flag.empty())
       continue;
     const Sample uma_id = GetSwitchUMAId(flag);
     EXPECT_NE(testing::kBadSwitchFormatHistogramId, uma_id)
         << "Command-line switch '" << flag
         << "' from about_flags.cc has UMA ID equal to reserved value "
            "kBadSwitchFormatHistogramId="
         << testing::kBadSwitchFormatHistogramId
         << ". Please modify switch name.";
     SwitchToIdMap::iterator enum_entry =
         histograms_xml_switches_ids.lower_bound(flag);
 
     // Ignore case here when switch ID is incorrect - it has already been
     // reported in the previous loop.
     EXPECT_TRUE(enum_entry != histograms_xml_switches_ids.end() &&
                 enum_entry->first == flag)
         << "histograms.xml enum LoginCustomFlags doesn't contain switch '"
         << flag << "' (value=" << uma_id
         << " expected). Consider adding entry:\n"
         << "  " << GetHistogramEnumEntryText(flag, uma_id);
   }
 }
 
 }  // namespace about_flags