Commit b6d5aa8f authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

chrome: put all flags into flag metadata

This change:

1) Adds every current flag to flag-metadata.json
2) Enables AboutFlagsTest.EveryFlagHasMetadata to ensure that no future flags
   without metadata are added

Note that every flag has been set to expire in M76 per
<https://sites.google.com/a/chromium.org/dev/flag-ownership>.

Bug: 897809
Change-Id: I67ab00e0462a50bb1a1484154f225c6e82d46097
Reviewed-on: https://chromium-review.googlesource.com/c/1335867
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608789}
parent b18d7062
...@@ -70,7 +70,7 @@ std::set<std::string> GetAllSwitchesAndFeaturesForTesting() { ...@@ -70,7 +70,7 @@ std::set<std::string> GetAllSwitchesAndFeaturesForTesting() {
struct FlagMetadataEntry { struct FlagMetadataEntry {
std::vector<std::string> owners; std::vector<std::string> owners;
int expire_milestone; int expiry_milestone;
}; };
using FlagMetadataMap = std::map<std::string, FlagMetadataEntry>; using FlagMetadataMap = std::map<std::string, FlagMetadataEntry>;
...@@ -92,10 +92,12 @@ FlagMetadataMap LoadFlagMetadata() { ...@@ -92,10 +92,12 @@ FlagMetadataMap LoadFlagMetadata() {
for (const auto& entry : metadata_json->GetList()) { for (const auto& entry : metadata_json->GetList()) {
std::string name = entry.FindKey("name")->GetString(); std::string name = entry.FindKey("name")->GetString();
std::vector<std::string> owners; std::vector<std::string> owners;
for (const auto& owner : entry.FindKey("owners")->GetList()) if (const base::Value* e = entry.FindKey("owners")) {
owners.push_back(owner.GetString()); for (const auto& owner : e->GetList())
int expire_milestone = entry.FindKey("expire_milestone")->GetInt(); owners.push_back(owner.GetString());
metadata[name] = FlagMetadataEntry{owners, expire_milestone}; }
int expiry_milestone = entry.FindKey("expiry_milestone")->GetInt();
metadata[name] = FlagMetadataEntry{owners, expiry_milestone};
} }
return metadata; return metadata;
...@@ -116,7 +118,7 @@ TEST(AboutFlagsTest, NoSeparators) { ...@@ -116,7 +118,7 @@ TEST(AboutFlagsTest, NoSeparators) {
// Makes sure that every flag has an owner and an expiry entry in // Makes sure that every flag has an owner and an expiry entry in
// flag-metadata.json. // flag-metadata.json.
TEST(AboutFlagsTest, DISABLED_EveryFlagHasMetadata) { TEST(AboutFlagsTest, EveryFlagHasMetadata) {
size_t count; size_t count;
const flags_ui::FeatureEntry* entries = testing::GetFeatureEntries(&count); const flags_ui::FeatureEntry* entries = testing::GetFeatureEntries(&count);
FlagMetadataMap metadata = LoadFlagMetadata(); FlagMetadataMap metadata = LoadFlagMetadata();
...@@ -128,10 +130,27 @@ TEST(AboutFlagsTest, DISABLED_EveryFlagHasMetadata) { ...@@ -128,10 +130,27 @@ TEST(AboutFlagsTest, DISABLED_EveryFlagHasMetadata) {
missing_flags.push_back(entries[i].internal_name); missing_flags.push_back(entries[i].internal_name);
} }
std::sort(missing_flags.begin(), missing_flags.end());
EXPECT_EQ(0u, missing_flags.size()) EXPECT_EQ(0u, missing_flags.size())
<< "Missing flags: " << base::JoinString(missing_flags, "\n "); << "Missing flags: " << base::JoinString(missing_flags, "\n ");
} }
TEST(AboutFlagsTest, DISABLED_EveryFlagHasNonEmptyOwners) {
FlagMetadataMap metadata = LoadFlagMetadata();
std::vector<std::string> sad_flags;
for (const auto& it : metadata) {
if (it.second.owners.empty())
sad_flags.push_back(it.first);
}
std::sort(sad_flags.begin(), sad_flags.end());
EXPECT_EQ(0u, sad_flags.size())
<< "Flags missing owners: " << base::JoinString(sad_flags, "\n ");
}
class AboutFlagsHistogramTest : public ::testing::Test { class AboutFlagsHistogramTest : public ::testing::Test {
protected: protected:
// This is a helper function to check that all IDs in enum LoginCustomFlags in // This is a helper function to check that all IDs in enum LoginCustomFlags in
......
This diff is collapsed.
...@@ -2834,6 +2834,7 @@ test("unit_tests") { ...@@ -2834,6 +2834,7 @@ test("unit_tests") {
] ]
data = [ data = [
"../browser/flag-metadata.json",
"data/", "data/",
"//base/test/data/", "//base/test/data/",
"//chrome/third_party/mock4js/", "//chrome/third_party/mock4js/",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment