Merge "aosp: reenable the filtered out test point in atest" into main

tools/aconfig/aconfig_storage_file/Android.bp

@@ -47,38 +47,14 @@ genrule {
     cmd: "rm -f $(out);cp -f $(in) $(out);chmod -w $(out)",
 }
 
-genrule {
-    name: "rw.package.map",
-    out: ["tests/tmp.rw.package.map"],
-    srcs: ["tests/package.map"],
-    cmd: "rm -f $(out);cp -f $(in) $(out);chmod +w $(out)",
-}
-
-genrule {
-    name: "rw.flag.map",
-    out: ["tests/tmp.rw.flag.map"],
-    srcs: ["tests/flag.map"],
-    cmd: "rm -f $(out);cp -f $(in) $(out);chmod +w $(out)",
-}
-
-genrule {
-    name: "rw.flag.val",
-    out: ["tests/tmp.rw.flag.val"],
-    srcs: ["tests/flag.val"],
-    cmd: "rm -f $(out);cp -f $(in) $(out);chmod +w $(out)",
-}
-
 rust_test_host {
     name: "aconfig_storage_file.test",
     test_suites: ["general-tests"],
     defaults: ["aconfig_storage_file.defaults"],
     data: [
-        ":ro.package.map",
+        "tests/package.map",
-        ":ro.flag.map",
+        "tests/flag.map",
-        ":ro.flag.val",
+        "tests/flag.val",
-        ":rw.package.map",
-        ":rw.flag.map",
-        ":rw.flag.val",
     ],
 }
 

tools/aconfig/aconfig_storage_file/src/lib.rs

@@ -438,32 +438,38 @@ impl ffi::BooleanFlagValueQueryCXX {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::test_utils::{
+    use crate::test_utils::{write_storage_text_to_temp_file, TestStorageFileSet};
-        create_temp_storage_files_for_test, get_binary_storage_proto_bytes,
+
-        set_temp_storage_files_to_read_only, write_bytes_to_temp_file,
+    fn create_test_storage_files(read_only: bool) -> TestStorageFileSet {
-    };
+        TestStorageFileSet::new(
+            "./tests/package.map",
+            "./tests/flag.map",
+            "./tests/flag.val",
+            read_only,
+        )
+        .unwrap()
+    }
 
     #[test]
     // this test point locks down flag package offset query
     fn test_package_offset_query() {
-        #[cfg(feature = "cargo")]
+        let ro_files = create_test_storage_files(true);
-        create_temp_storage_files_for_test();
+        let text_proto = format!(
-
+            r#"
-        set_temp_storage_files_to_read_only();
+files {{
-        let text_proto = r#"
-files {
     version: 0
     container: "system"
-    package_map: "./tests/tmp.ro.package.map"
+    package_map: "{}"
-    flag_map: "./tests/tmp.ro.flag.map"
+    flag_map: "{}"
-    flag_val: "./tests/tmp.ro.flag.val"
+    flag_val: "{}"
     timestamp: 12345
-}
+}}
-"#;
+"#,
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
+            ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
-        let file = write_bytes_to_temp_file(&binary_proto_bytes).unwrap();
+        );
-        let file_full_path = file.path().display().to_string();
 
+        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file_full_path = file.path().display().to_string();
         let package_offset = get_package_offset_impl(
             &file_full_path,
             "system",
@@ -498,24 +504,23 @@ files {
     #[test]
     // this test point locks down flag offset query
     fn test_flag_offset_query() {
-        #[cfg(feature = "cargo")]
+        let ro_files = create_test_storage_files(true);
-        create_temp_storage_files_for_test();
+        let text_proto = format!(
-
+            r#"
-        set_temp_storage_files_to_read_only();
+files {{
-        let text_proto = r#"
-files {
     version: 0
     container: "system"
-    package_map: "./tests/tmp.ro.package.map"
+    package_map: "{}"
-    flag_map: "./tests/tmp.ro.flag.map"
+    flag_map: "{}"
-    flag_val: "./tests/tmp.ro.flag.val"
+    flag_val: "{}"
     timestamp: 12345
-}
+}}
-"#;
+"#,
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
+            ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
-        let file = write_bytes_to_temp_file(&binary_proto_bytes).unwrap();
+        );
-        let file_full_path = file.path().display().to_string();
 
+        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file_full_path = file.path().display().to_string();
         let baseline = vec![
             (0, "enabled_ro", 1u16),
             (0, "enabled_rw", 2u16),
@@ -538,24 +543,23 @@ files {
     #[test]
     // this test point locks down flag offset query
     fn test_flag_value_query() {
-        #[cfg(feature = "cargo")]
+        let ro_files = create_test_storage_files(true);
-        create_temp_storage_files_for_test();
+        let text_proto = format!(
-
+            r#"
-        set_temp_storage_files_to_read_only();
+files {{
-        let text_proto = r#"
-files {
     version: 0
     container: "system"
-    package_map: "./tests/tmp.ro.package.map"
+    package_map: "{}"
-    flag_map: "./tests/tmp.ro.flag.map"
+    flag_map: "{}"
-    flag_val: "./tests/tmp.ro.flag.val"
+    flag_val: "{}"
     timestamp: 12345
-}
+}}
-"#;
+"#,
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
+            ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
-        let file = write_bytes_to_temp_file(&binary_proto_bytes).unwrap();
+        );
-        let file_full_path = file.path().display().to_string();
 
+        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file_full_path = file.path().display().to_string();
         let baseline: Vec<bool> = vec![false; 8];
         for (offset, expected_value) in baseline.into_iter().enumerate() {
             let flag_value =

tools/aconfig/aconfig_storage_file/src/mapped_file.rs

@@ -148,10 +148,7 @@ pub(crate) fn get_mapped_file(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::test_utils::{
+    use crate::test_utils::{write_storage_text_to_temp_file, TestStorageFileSet};
-        create_temp_storage_files_for_test, get_binary_storage_proto_bytes,
-        set_temp_storage_files_to_read_only, write_bytes_to_temp_file,
-    };
 
     #[test]
     fn test_find_storage_file_location() {
@@ -173,10 +170,8 @@ files {
     timestamp: 54321
 }
 "#;
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
+        let file = write_storage_text_to_temp_file(text_proto).unwrap();
-        let file = write_bytes_to_temp_file(&binary_proto_bytes).unwrap();
         let file_full_path = file.path().display().to_string();
-
         let file_info = find_container_storage_location(&file_full_path, "system").unwrap();
         assert_eq!(file_info.version(), 0);
         assert_eq!(file_info.container(), "system");
@@ -213,100 +208,121 @@ files {
         assert_eq!(mmaped_file[..], content[..]);
     }
 
+    fn create_test_storage_files(read_only: bool) -> TestStorageFileSet {
+        TestStorageFileSet::new(
+            "./tests/package.map",
+            "./tests/flag.map",
+            "./tests/flag.val",
+            read_only,
+        )
+        .unwrap()
+    }
+
     #[test]
     fn test_mapped_file_contents() {
-        #[cfg(feature = "cargo")]
+        let ro_files = create_test_storage_files(true);
-        create_temp_storage_files_for_test();
+        let text_proto = format!(
-
+            r#"
-        set_temp_storage_files_to_read_only();
+files {{
-        let text_proto = r#"
-files {
     version: 0
     container: "system"
-    package_map: "./tests/tmp.ro.package.map"
+    package_map: "{}"
-    flag_map: "./tests/tmp.ro.flag.map"
+    flag_map: "{}"
-    flag_val: "./tests/tmp.ro.flag.val"
+    flag_val: "{}"
     timestamp: 12345
-}
+}}
-"#;
+"#,
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
+            ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
-        let file = write_bytes_to_temp_file(&binary_proto_bytes).unwrap();
+        );
-        let file_full_path = file.path().display().to_string();
 
+        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file_full_path = file.path().display().to_string();
         map_and_verify(
             &file_full_path,
             StorageFileSelection::PackageMap,
-            "./tests/tmp.ro.package.map",
+            &ro_files.package_map.name,
         );
-        map_and_verify(&file_full_path, StorageFileSelection::FlagMap, "./tests/tmp.ro.flag.map");
+        map_and_verify(&file_full_path, StorageFileSelection::FlagMap, &ro_files.flag_map.name);
-        map_and_verify(&file_full_path, StorageFileSelection::FlagVal, "./tests/tmp.ro.flag.val");
+        map_and_verify(&file_full_path, StorageFileSelection::FlagVal, &ro_files.flag_val.name);
     }
 
     #[test]
-    #[cfg(feature = "cargo")]
     fn test_map_non_read_only_file() {
-        #[cfg(feature = "cargo")]
+        let ro_files = create_test_storage_files(true);
-        create_temp_storage_files_for_test();
+        let rw_files = create_test_storage_files(false);
-
+        let text_proto = format!(
-        set_temp_storage_files_to_read_only();
+            r#"
-        let text_proto = r#"
+files {{
-files {
     version: 0
     container: "system"
-    package_map: "./tests/tmp.rw.package.map"
+    package_map: "{}"
-    flag_map: "./tests/tmp.rw.flag.map"
+    flag_map: "{}"
-    flag_val: "./tests/tmp.rw.flag.val"
+    flag_val: "{}"
     timestamp: 12345
-}
+}}
-"#;
+"#,
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
+            rw_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
-        let file = write_bytes_to_temp_file(&binary_proto_bytes).unwrap();
-        let file_full_path = file.path().display().to_string();
-
-        let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
-        assert_eq!(
-            format!("{:?}", error),
-            "MapFileFail(fail to map non read only storage file ./tests/tmp.rw.package.map)"
         );
 
-        let text_proto = r#"
+        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
-files {
-    version: 0
-    container: "system"
-    package_map: "./tests/tmp.ro.package.map"
-    flag_map: "./tests/tmp.rw.flag.map"
-    flag_val: "./tests/tmp.rw.flag.val"
-    timestamp: 12345
-}
-"#;
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
-        let file = write_bytes_to_temp_file(&binary_proto_bytes).unwrap();
         let file_full_path = file.path().display().to_string();
-
         let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
         assert_eq!(
             format!("{:?}", error),
-            "MapFileFail(fail to map non read only storage file ./tests/tmp.rw.flag.map)"
+            format!(
+                "MapFileFail(fail to map non read only storage file {})",
+                rw_files.package_map.name
+            )
         );
 
-        let text_proto = r#"
+        let text_proto = format!(
-files {
+            r#"
+files {{
     version: 0
     container: "system"
-    package_map: "./tests/tmp.ro.package.map"
+    package_map: "{}"
-    flag_map: "./tests/tmp.ro.flag.map"
+    flag_map: "{}"
-    flag_val: "./tests/tmp.rw.flag.val"
+    flag_val: "{}"
     timestamp: 12345
-}
+}}
-"#;
+"#,
-        let binary_proto_bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
+            ro_files.package_map.name, rw_files.flag_map.name, ro_files.flag_val.name
-        let file = write_bytes_to_temp_file(&binary_proto_bytes).unwrap();
+        );
-        let file_full_path = file.path().display().to_string();
 
+        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file_full_path = file.path().display().to_string();
         let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
         assert_eq!(
             format!("{:?}", error),
-            "MapFileFail(fail to map non read only storage file ./tests/tmp.rw.flag.val)"
+            format!(
+                "MapFileFail(fail to map non read only storage file {})",
+                rw_files.flag_map.name
+            )
+        );
+
+        let text_proto = format!(
+            r#"
+files {{
+    version: 0
+    container: "system"
+    package_map: "{}"
+    flag_map: "{}"
+    flag_val: "{}"
+    timestamp: 12345
+}}
+"#,
+            ro_files.package_map.name, ro_files.flag_map.name, rw_files.flag_val.name
+        );
+
+        let file = write_storage_text_to_temp_file(&text_proto).unwrap();
+        let file_full_path = file.path().display().to_string();
+        let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
+        assert_eq!(
+            format!("{:?}", error),
+            format!(
+                "MapFileFail(fail to map non read only storage file {})",
+                rw_files.flag_val.name
+            )
         );
     }
 }

tools/aconfig/aconfig_storage_file/src/test_utils.rs

@@ -19,12 +19,8 @@ use anyhow::Result;
 use protobuf::Message;
 use std::fs;
 use std::io::Write;
-use std::path::Path;
-use std::sync::Once;
 use tempfile::NamedTempFile;
 
-static INIT: Once = Once::new();
-
 pub(crate) fn get_binary_storage_proto_bytes(text_proto: &str) -> Result<Vec<u8>> {
     let storage_files: ProtoStorageFiles = protobuf::text_format::parse_from_str(text_proto)?;
     let mut binary_proto = Vec::new();
@@ -32,59 +28,65 @@ pub(crate) fn get_binary_storage_proto_bytes(text_proto: &str) -> Result<Vec<u8>
     Ok(binary_proto)
 }
 
-pub(crate) fn write_bytes_to_temp_file(bytes: &[u8]) -> Result<NamedTempFile> {
+pub(crate) fn write_storage_text_to_temp_file(text_proto: &str) -> Result<NamedTempFile> {
+    let bytes = get_binary_storage_proto_bytes(text_proto).unwrap();
     let mut file = NamedTempFile::new()?;
     let _ = file.write_all(&bytes);
     Ok(file)
 }
 
-fn has_same_content(file1: &Path, file2: &Path) -> Result<bool> {
+fn set_file_read_only(file: &NamedTempFile) {
-    let bytes1 = fs::read(file1)?;
+    let mut perms = fs::metadata(file.path()).unwrap().permissions();
-    let bytes2 = fs::read(file2)?;
+    if !perms.readonly() {
-    if bytes1.len() != bytes2.len() {
+        perms.set_readonly(true);
-        return Ok(false);
+        fs::set_permissions(file.path(), perms).unwrap();
     }
-    for (i, &b1) in bytes1.iter().enumerate() {
-        if b1 != bytes2[i] {
-            return Ok(false);
-        }
-    }
-    Ok(true)
 }
 
-pub(crate) fn create_temp_storage_files_for_test() {
+fn set_file_read_write(file: &NamedTempFile) {
-    INIT.call_once(|| {
+    let mut perms = fs::metadata(file.path()).unwrap().permissions();
-        let file_paths = [
+    if perms.readonly() {
-            ("./tests/package.map", "./tests/tmp.ro.package.map"),
+        perms.set_readonly(false);
-            ("./tests/flag.map", "./tests/tmp.ro.flag.map"),
+        fs::set_permissions(file.path(), perms).unwrap();
-            ("./tests/flag.val", "./tests/tmp.ro.flag.val"),
+    }
-            ("./tests/package.map", "./tests/tmp.rw.package.map"),
+}
-            ("./tests/flag.map", "./tests/tmp.rw.flag.map"),
+
-            ("./tests/flag.val", "./tests/tmp.rw.flag.val"),
+pub(crate) struct TestStorageFile {
-        ];
+    pub file: NamedTempFile,
-        for (file_path, copied_file_path) in file_paths.into_iter() {
+    pub name: String,
-            let file_path = Path::new(&file_path);
+}
-            let copied_file_path = Path::new(&copied_file_path);
+
-            if copied_file_path.exists() && !has_same_content(file_path, copied_file_path).unwrap()
+impl TestStorageFile {
-            {
+    pub(crate) fn new(source_file: &str, read_only: bool) -> Result<Self> {
-                fs::remove_file(copied_file_path).unwrap();
+        let file = NamedTempFile::new()?;
-            }
+        fs::copy(source_file, file.path())?;
-            if !copied_file_path.exists() {
+        if read_only {
-                fs::copy(file_path, copied_file_path).unwrap();
+            set_file_read_only(&file);
-            }
+        } else {
-        }
+            set_file_read_write(&file);
-    });
+        }
-}
+        let name = file.path().display().to_string();
-
+        Ok(Self { file, name })
-pub(crate) fn set_temp_storage_files_to_read_only() {
+    }
-    let file_paths =
+}
-        ["./tests/tmp.ro.package.map", "./tests/tmp.ro.flag.map", "./tests/tmp.ro.flag.val"];
+
-    for file_path in file_paths.into_iter() {
+pub(crate) struct TestStorageFileSet {
-        let file_path = Path::new(&file_path);
+    pub package_map: TestStorageFile,
-        let mut perms = fs::metadata(file_path).unwrap().permissions();
+    pub flag_map: TestStorageFile,
-        if !perms.readonly() {
+    pub flag_val: TestStorageFile,
-            perms.set_readonly(true);
+}
-            fs::set_permissions(file_path, perms).unwrap();
+
-        }
+impl TestStorageFileSet {
+    pub(crate) fn new(
+        package_map_path: &str,
+        flag_map_path: &str,
+        flag_val_path: &str,
+        read_only: bool,
+    ) -> Result<Self> {
+        Ok(Self {
+            package_map: TestStorageFile::new(package_map_path, read_only)?,
+            flag_map: TestStorageFile::new(flag_map_path, read_only)?,
+            flag_val: TestStorageFile::new(flag_val_path, read_only)?,
+        })
     }
 }