Merge "aconfig: require exactly one bug field" into main
diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs
index 4ddada7..2ab6e05 100644
--- a/tools/aconfig/src/protos.rs
+++ b/tools/aconfig/src/protos.rs
@@ -92,8 +92,7 @@
ensure!(codegen::is_valid_name_ident(pdf.name()), "bad flag declaration: bad name");
ensure!(codegen::is_valid_name_ident(pdf.namespace()), "bad flag declaration: bad name");
ensure!(!pdf.description().is_empty(), "bad flag declaration: empty description");
-
- // ProtoFlagDeclaration.bug: Vec<String>: may be empty, no checks needed
+ ensure!(pdf.bug.len() == 1, "bad flag declaration: exactly one bug required");
Ok(())
}
@@ -195,8 +194,7 @@
for tp in pf.trace.iter() {
super::tracepoint::verify_fields(tp)?;
}
-
- // ProtoParsedFlag.bug: Vec<String>: may be empty, no checks needed
+ ensure!(pf.bug.len() == 1, "bad flag declaration: exactly one bug required");
Ok(())
}
@@ -279,12 +277,12 @@
namespace: "first_ns"
description: "This is the description of the first flag."
bug: "123"
- bug: "abc"
}
flag {
name: "second"
namespace: "second_ns"
description: "This is the description of the second flag."
+ bug: "abc"
}
"#,
)
@@ -294,14 +292,12 @@
assert_eq!(first.name(), "first");
assert_eq!(first.namespace(), "first_ns");
assert_eq!(first.description(), "This is the description of the first flag.");
- assert_eq!(first.bug.len(), 2);
- assert_eq!(first.bug[0], "123");
- assert_eq!(first.bug[1], "abc");
+ assert_eq!(first.bug, vec!["123"]);
let second = flag_declarations.flag.iter().find(|pf| pf.name() == "second").unwrap();
assert_eq!(second.name(), "second");
assert_eq!(second.namespace(), "second_ns");
assert_eq!(second.description(), "This is the description of the second flag.");
- assert_eq!(second.bug.len(), 0);
+ assert_eq!(second.bug, vec!["abc"]);
// bad input: missing package in flag declarations
let error = flag_declarations::try_from_text_proto(
@@ -376,6 +372,36 @@
)
.unwrap_err();
assert!(format!("{:?}", error).contains("bad flag declaration: bad name"));
+
+ // bad input: no bug entries in flag declaration
+ let error = flag_declarations::try_from_text_proto(
+ r#"
+package: "com.foo.bar"
+flag {
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("bad flag declaration: exactly one bug required"));
+
+ // bad input: multiple bug entries in flag declaration
+ let error = flag_declarations::try_from_text_proto(
+ r#"
+package: "com.foo.bar"
+flag {
+ name: "first"
+ namespace: "first_ns"
+ description: "This is the description of the first flag."
+ bug: "123"
+ bug: "abc"
+}
+"#,
+ )
+ .unwrap_err();
+ assert!(format!("{:?}", error).contains("bad flag declaration: exactly one bug required"));
}
#[test]
@@ -482,6 +508,7 @@
name: "first"
namespace: "first_ns"
description: "This is the description of the first flag."
+ bug: "SOME_BUG"
state: DISABLED
permission: READ_ONLY
trace {
@@ -495,6 +522,7 @@
name: "second"
namespace: "second_ns"
description: "This is the description of the second flag."
+ bug: "SOME_BUG"
state: ENABLED
permission: READ_WRITE
trace {
@@ -516,6 +544,7 @@
assert_eq!(second.name(), "second");
assert_eq!(second.namespace(), "second_ns");
assert_eq!(second.description(), "This is the description of the second flag.");
+ assert_eq!(second.bug, vec!["SOME_BUG"]);
assert_eq!(second.state(), ProtoFlagState::ENABLED);
assert_eq!(second.permission(), ProtoFlagPermission::READ_WRITE);
assert_eq!(2, second.trace.len());
@@ -569,6 +598,7 @@
name: "first"
namespace: "first_ns"
description: "This is the description of the first flag."
+ bug: ""
state: DISABLED
permission: READ_ONLY
trace {
@@ -582,6 +612,7 @@
name: "second"
namespace: "second_ns"
description: "This is the description of the second flag."
+ bug: ""
state: ENABLED
permission: READ_WRITE
trace {
@@ -604,6 +635,7 @@
name: "bbb"
namespace: "first_ns"
description: "This is the description of the first flag."
+ bug: ""
state: DISABLED
permission: READ_ONLY
trace {
@@ -617,6 +649,7 @@
name: "aaa"
namespace: "second_ns"
description: "This is the description of the second flag."
+ bug: ""
state: ENABLED
permission: READ_WRITE
trace {
@@ -639,6 +672,7 @@
name: "bar"
namespace: "first_ns"
description: "This is the description of the first flag."
+ bug: ""
state: DISABLED
permission: READ_ONLY
trace {
@@ -652,6 +686,7 @@
name: "bar"
namespace: "second_ns"
description: "This is the description of the second flag."
+ bug: ""
state: ENABLED
permission: READ_WRITE
trace {
@@ -673,6 +708,7 @@
name: "bar"
namespace: "first_ns"
description: "This is the description of the first flag."
+ bug: "b/12345678"
state: DISABLED
permission: READ_ONLY
trace {
@@ -703,6 +739,7 @@
name: "first"
namespace: "first_ns"
description: "This is the description of the first flag."
+ bug: "a"
state: DISABLED
permission: READ_ONLY
trace {
@@ -716,6 +753,7 @@
name: "second"
namespace: "second_ns"
description: "This is the description of the second flag."
+ bug: "b"
state: ENABLED
permission: READ_WRITE
trace {
@@ -733,6 +771,7 @@
name: "first"
namespace: "first_ns"
description: "This is the description of the first flag."
+ bug: "a"
state: DISABLED
permission: READ_ONLY
trace {
@@ -749,6 +788,7 @@
package: "com.second"
name: "second"
namespace: "second_ns"
+ bug: "b"
description: "This is the description of the second flag."
state: ENABLED
permission: READ_WRITE
diff --git a/tools/aconfig/src/test.rs b/tools/aconfig/src/test.rs
index 04bbe28..14beb93 100644
--- a/tools/aconfig/src/test.rs
+++ b/tools/aconfig/src/test.rs
@@ -61,7 +61,6 @@
name: "enabled_ro"
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_ONLY"
- bug: "789"
bug: "abc"
state: ENABLED
permission: READ_ONLY
@@ -86,6 +85,7 @@
name: "enabled_rw"
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_WRITE"
+ bug: ""
state: ENABLED
permission: READ_WRITE
trace {
diff --git a/tools/aconfig/tests/test.aconfig b/tools/aconfig/tests/test.aconfig
index d7ac919..46cf1e9 100644
--- a/tools/aconfig/tests/test.aconfig
+++ b/tools/aconfig/tests/test.aconfig
@@ -8,7 +8,6 @@
name: "enabled_ro"
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_ONLY"
- bug: "789"
bug: "abc"
}
@@ -19,7 +18,8 @@
name: "enabled_rw"
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_WRITE"
- # no bug field: bug is not mandatory
+ # for bug fields, the empty string is a discouraged but valid value
+ bug: ""
}
# This flag's final value is calculated from: