[flashrom-gerrit] [M] Change in flashrom[master]: Revert "flashrom_tester: Add positive check to verify_fail_test"

[Edward O'Callaghan (Code Review)]

Attention is currently required from: Peter Marheine, Evan Benn.

Hello build bot (Jenkins), Evan Benn, Peter Marheine,

I'd like you to do a code review.
Please visit

https://review.coreboot.org/c/flashrom/+/72674

to review the following change.

Change subject: Revert "flashrom_tester: Add positive check to verify_fail_test"
......................................................................

Revert "flashrom_tester: Add positive check to verify_fail_test"

This reverts commit 5735529d6209388079340a6fbd55e222a1c4c1b9.

Reason for revert: Fails to type-check in verify_region_from_file().

Change-Id: Ib8c99de7aca672f42b3159082e2e63e0fefef08f
---
M util/flashrom_tester/flashrom/src/cmd.rs
M util/flashrom_tester/flashrom/src/flashromlib.rs
M util/flashrom_tester/flashrom/src/lib.rs
M util/flashrom_tester/src/tests.rs
4 files changed, 15 insertions(+), 50 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/72674/1

diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs
index 1f13a8e..85cbb12 100644
--- a/util/flashrom_tester/flashrom/src/cmd.rs
+++ b/util/flashrom_tester/flashrom/src/cmd.rs
@@ -269,18 +269,6 @@
         Ok(())
     }

-    fn verify_region_from_file(&self, path: &Path, region: &str) -> Result<(), FlashromError> {
-        let opts = FlashromOpt {
-            io_opt: Some(IOOpt::Verify(OperationArgs::RegionFileRegion(
-                region, path, None,
-            ))),
-            ..Default::default()
-        };
-
-        self.dispatch(opts, "verify_region_from_file")?;
-        Ok(())
-    }
-
     fn erase(&self) -> Result<(), FlashromError> {
         let opts = FlashromOpt {
             io_opt: Some(IOOpt::Erase),
diff --git a/util/flashrom_tester/flashrom/src/flashromlib.rs b/util/flashrom_tester/flashrom/src/flashromlib.rs
index 5e1747b..bf09e6d 100644
--- a/util/flashrom_tester/flashrom/src/flashromlib.rs
+++ b/util/flashrom_tester/flashrom/src/flashromlib.rs
@@ -152,27 +152,6 @@
         Ok(())
     }

-    fn verify_region_from_file(&self, path: &Path, region: &str) -> Result<(), FlashromError> {
-        let mut layout = self.flashrom.borrow_mut().layout_read_fmap_from_rom()?;
-        layout.include_region(region)?;
-        let range = layout.get_region_range(region)?;
-        let region_data = fs::read(path).map_err(|error| error.to_string())?;
-        if region_data.len() != range.len() {
-            return Err(format!(
-                "verify region range ({}) does not match provided file size ({})",
-                range.len(),
-                region_data.len()
-            )
-            .into());
-        }
-        let mut buf = vec![0; self.get_size()? as usize];
-        buf[range].copy_from_slice(&region_data);
-        self.flashrom
-            .borrow_mut()
-            .image_verify(&buf, Some(layout))?;
-        Ok(())
-    }
-
     fn erase(&self) -> Result<(), FlashromError> {
         self.flashrom.borrow_mut().erase()?;
         Ok(())
diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs
index 41393e8..7c86649 100644
--- a/util/flashrom_tester/flashrom/src/lib.rs
+++ b/util/flashrom_tester/flashrom/src/lib.rs
@@ -133,8 +133,7 @@
     /// Read the whole flash to the file specified by `path`.
     fn read_into_file(&self, path: &Path) -> Result<(), FlashromError>;

-    /// Read only a region of the flash into the file specified by `path`. Note
-    /// the first byte written to the file is the first byte from the region.
+    /// Read only a region of the flash.
     fn read_region_into_file(&self, path: &Path, region: &str) -> Result<(), FlashromError>;

/// Write the whole flash to the file specified by `path`.
@@ -153,10 +152,6 @@
     /// Verify the whole flash against the file specified by `path`.
     fn verify_from_file(&self, path: &Path) -> Result<(), FlashromError>;

-    /// Verify only the region against the file specified by `path`.
-    /// Note the first byte in the file is matched against the first byte of the region.
-    fn verify_region_from_file(&self, path: &Path, region: &str) -> Result<(), FlashromError>;
-
     /// Erase the whole flash.
     fn erase(&self) -> Result<(), FlashromError>;

diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs
index 721a789..847bfec 100644
--- a/util/flashrom_tester/src/tests.rs
+++ b/util/flashrom_tester/src/tests.rs
@@ -44,7 +44,6 @@
 use std::sync::atomic::AtomicBool;

const ELOG_FILE: &str = "/tmp/elog.file";
-const FW_MAIN_B_PATH: &str = "/tmp/FW_MAIN_B.bin";

/// Iterate over tests, yielding only those tests with names matching filter_names.
 ///
@@ -313,16 +312,7 @@

/// Check that flashrom 'verify' will fail if the provided data does not match the chip data.
 fn verify_fail_test(env: &mut TestEnv) -> TestResult {
-    env.ensure_golden()?;
-    // Verify that verify is Ok when the data matches. We verify only a region
-    // and not the whole chip because coprocessors or firmware may have written
-    // some data in other regions.
-    env.cmd
-        .read_region_into_file(FW_MAIN_B_PATH.as_ref(), "FW_MAIN_B")?;
-    env.cmd
-        .verify_region_from_file(FW_MAIN_B_PATH.as_ref(), "FW_MAIN_B")?;
-
-    // Verify that verify is false when the data does not match
+    // Comparing the flash contents to random data says they're not the same.
     match env.verify(env.random_data_file()) {
         Ok(_) => Err("Verification says flash is full of random data".into()),
         Err(_) => Ok(()),