Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(52)

Side by Side Diff: chrome/browser/about_flags_unittest.cc

Issue 2345033002: Log base::Features in Launch.FlagsAtStartup (Closed)
Patch Set: Clean up test and localize ReportAboutFlagsHistogram{Switches, Features} Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2011 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/about_flags.h" 5 #include "chrome/browser/about_flags.h"
6 6
7 #include <stddef.h> 7 #include <stddef.h>
8 8
9 #include <map> 9 #include <map>
10 #include <set> 10 #include <set>
(...skipping 150 matching lines...) Expand 10 before | Expand all | Expand 10 after
161 161
162 std::string FilePathStringTypeToString(const base::FilePath::StringType& path) { 162 std::string FilePathStringTypeToString(const base::FilePath::StringType& path) {
163 #if defined(OS_WIN) 163 #if defined(OS_WIN)
164 return base::UTF16ToUTF8(path); 164 return base::UTF16ToUTF8(path);
165 #else 165 #else
166 return path; 166 return path;
167 #endif 167 #endif
168 } 168 }
169 169
170 // Get all associated switches corresponding to defined about_flags.cc entries. 170 // Get all associated switches corresponding to defined about_flags.cc entries.
171 // Does not include information about FEATURE_VALUE or 171 // Does not include information about FEATURE_WITH_VARIATIONS_VALUE entries.
172 // FEATURE_WITH_VARIATIOSN_VALUE entries. 172 std::set<std::string> GetAllSwitchesAndFeaturesForTesting() {
173 std::set<std::string> GetAllSwitchesForTesting() {
174 std::set<std::string> result; 173 std::set<std::string> result;
175 174
176 size_t num_entries = 0; 175 size_t num_entries = 0;
177 const flags_ui::FeatureEntry* entries = 176 const flags_ui::FeatureEntry* entries =
178 testing::GetFeatureEntries(&num_entries); 177 testing::GetFeatureEntries(&num_entries);
179 178
180 for (size_t i = 0; i < num_entries; ++i) { 179 for (size_t i = 0; i < num_entries; ++i) {
181 const flags_ui::FeatureEntry& entry = entries[i]; 180 const flags_ui::FeatureEntry& entry = entries[i];
182 switch (entry.type) { 181 switch (entry.type) {
183 case flags_ui::FeatureEntry::SINGLE_VALUE: 182 case flags_ui::FeatureEntry::SINGLE_VALUE:
184 case flags_ui::FeatureEntry::SINGLE_DISABLE_VALUE: 183 case flags_ui::FeatureEntry::SINGLE_DISABLE_VALUE:
185 result.insert(entry.command_line_switch); 184 result.insert(entry.command_line_switch);
186 break; 185 break;
187 case flags_ui::FeatureEntry::MULTI_VALUE: 186 case flags_ui::FeatureEntry::MULTI_VALUE:
188 for (int j = 0; j < entry.num_options; ++j) { 187 for (int j = 0; j < entry.num_options; ++j) {
189 result.insert(entry.ChoiceForOption(j).command_line_switch); 188 result.insert(entry.ChoiceForOption(j).command_line_switch);
190 } 189 }
191 break; 190 break;
192 case flags_ui::FeatureEntry::ENABLE_DISABLE_VALUE: 191 case flags_ui::FeatureEntry::ENABLE_DISABLE_VALUE:
193 result.insert(entry.command_line_switch); 192 result.insert(entry.command_line_switch);
194 result.insert(entry.disable_command_line_switch); 193 result.insert(entry.disable_command_line_switch);
195 break; 194 break;
196 case flags_ui::FeatureEntry::FEATURE_VALUE: 195 case flags_ui::FeatureEntry::FEATURE_VALUE:
196 result.insert(std::string(entry.feature->name) + ":enabled");
197 result.insert(std::string(entry.feature->name) + ":disabled");
198 break;
197 case flags_ui::FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE: 199 case flags_ui::FeatureEntry::FEATURE_WITH_VARIATIONS_VALUE:
Alexei Svitkine (slow) 2016/09/16 20:50:33 I think it's fine to handle this same as the above
lawrencewu 2016/09/16 21:20:03 Done.
198 break; 200 break;
199 } 201 }
200 } 202 }
201 return result; 203 return result;
202 } 204 }
203 205
204 } // anonymous namespace 206 } // anonymous namespace
205 207
206 // Makes sure there are no separators in any of the entry names. 208 // Makes sure there are no separators in any of the entry names.
207 TEST(AboutFlagsTest, NoSeparators) { 209 TEST(AboutFlagsTest, NoSeparators) {
(...skipping 68 matching lines...) Expand 10 before | Expand all | Expand 10 after
276 EXPECT_EQ(uma_id, entry.first) 278 EXPECT_EQ(uma_id, entry.first)
277 << "histograms.xml enum LoginCustomFlags " 279 << "histograms.xml enum LoginCustomFlags "
278 "entry '" << entry.second << "' has incorrect value=" << entry.first 280 "entry '" << entry.second << "' has incorrect value=" << entry.first
279 << ", but " << uma_id << " is expected. Consider changing entry to:\n" 281 << ", but " << uma_id << " is expected. Consider changing entry to:\n"
280 << " " << GetHistogramEnumEntryText(entry.second, uma_id); 282 << " " << GetHistogramEnumEntryText(entry.second, uma_id);
281 SetSwitchToHistogramIdMapping(entry.second, entry.first, 283 SetSwitchToHistogramIdMapping(entry.second, entry.first,
282 &histograms_xml_switches_ids); 284 &histograms_xml_switches_ids);
283 } 285 }
284 286
285 // Check that all flags in about_flags.cc have entries in login_custom_flags. 287 // Check that all flags in about_flags.cc have entries in login_custom_flags.
286 std::set<std::string> all_switches = GetAllSwitchesForTesting(); 288 std::set<std::string> all_flags = GetAllSwitchesAndFeaturesForTesting();
287 for (const std::string& flag : all_switches) { 289 for (const std::string& flag : all_flags) {
288 // Skip empty placeholders. 290 // Skip empty placeholders.
289 if (flag.empty()) 291 if (flag.empty())
290 continue; 292 continue;
291 const Sample uma_id = GetSwitchUMAId(flag); 293 const Sample uma_id = GetSwitchUMAId(flag);
292 EXPECT_NE(testing::kBadSwitchFormatHistogramId, uma_id) 294 EXPECT_NE(testing::kBadSwitchFormatHistogramId, uma_id)
293 << "Command-line switch '" << flag 295 << "Command-line switch '" << flag
294 << "' from about_flags.cc has UMA ID equal to reserved value " 296 << "' from about_flags.cc has UMA ID equal to reserved value "
295 "kBadSwitchFormatHistogramId=" 297 "kBadSwitchFormatHistogramId="
296 << testing::kBadSwitchFormatHistogramId 298 << testing::kBadSwitchFormatHistogramId
297 << ". Please modify switch name."; 299 << ". Please modify switch name.";
298 SwitchToIdMap::iterator enum_entry = 300 SwitchToIdMap::iterator enum_entry =
299 histograms_xml_switches_ids.lower_bound(flag); 301 histograms_xml_switches_ids.lower_bound(flag);
300 302
301 // Ignore case here when switch ID is incorrect - it has already been 303 // Ignore case here when switch ID is incorrect - it has already been
302 // reported in the previous loop. 304 // reported in the previous loop.
303 EXPECT_TRUE(enum_entry != histograms_xml_switches_ids.end() && 305 EXPECT_TRUE(enum_entry != histograms_xml_switches_ids.end() &&
304 enum_entry->first == flag) 306 enum_entry->first == flag)
305 << "histograms.xml enum LoginCustomFlags doesn't contain switch '" 307 << "histograms.xml enum LoginCustomFlags doesn't contain switch '"
306 << flag << "' (value=" << uma_id 308 << flag << "' (value=" << uma_id
307 << " expected). Consider adding entry:\n" 309 << " expected). Consider adding entry:\n"
308 << " " << GetHistogramEnumEntryText(flag, uma_id); 310 << " " << GetHistogramEnumEntryText(flag, uma_id);
309 } 311 }
310 } 312 }
311 313
312 } // namespace about_flags 314 } // namespace about_flags
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698