Refactor parse_samples/parse_wild_samples to reduce duplication and add known failures

tests/parse_samples.rs

@@ -5,61 +5,64 @@ use pretty_assertions::assert_eq;
 
 use gitpatch::Patch;
 
-#[test]
-fn parse_samples() {
-    let samples_path = PathBuf::from(file!()).parent().unwrap().join("samples");
+fn visit_patches<F>(test_dir: &str, extension: &str, f: F)
+where
+    F: Fn(&str, &PathBuf),
+{
+    let samples_path = PathBuf::from(file!()).parent().unwrap().join(test_dir);
     for file in fs::read_dir(samples_path).unwrap() {
         let path = file.unwrap().path();
-        if path.extension().unwrap_or_default() != "diff" {
+        if path.extension().unwrap_or_default() != extension {
             continue;
         }
 
-        let data = fs::read_to_string(dbg!(&path)).unwrap();
-        let patches = Patch::from_multiple(&data)
-            .unwrap_or_else(|err| panic!("failed to parse {:?}, error: {}", path, err));
-
-        // Make sure that the patch file we produce parses to the same information as the original
-        // patch file.
-        #[allow(clippy::format_collect)] // Display::fmt is the only way to resolve Patch->str
-        let patch_file: String = patches.iter().map(|patch| format!("{}\n", patch)).collect();
-        println!("{}", patch_file);
-        let patches2 = Patch::from_multiple(&patch_file).unwrap_or_else(|err| {
-            panic!(
-                "failed to re-parse {:?} after formatting, error: {}",
-                path, err
-            )
-        });
-        assert_eq!(patches, patches2);
+        f(fs::read_to_string(dbg!(&path)).unwrap().as_str(), &path);
     }
 }
 
+// Make sure that the patch file we produce parses to the same information as the original
+// patch file.
+fn verify_patch_roundtrip(data: &str, path: &PathBuf) {
+    let patches = Patch::from_multiple(data)
+        .unwrap_or_else(|err| panic!("failed to parse {:?}, error: {}", path, err));
+
+    #[allow(clippy::format_collect)] // Display::fmt is the only way to resolve Patch->str
+    let patch_file: String = patches.iter().map(|patch| format!("{}\n", patch)).collect();
+    println!("{}", patch_file);
+    let patches2 = Patch::from_multiple(&patch_file).unwrap_or_else(|err| {
+        panic!(
+            "failed to re-parse {:?} after formatting, error: {}",
+            path, err
+        )
+    });
+    assert_eq!(patches, patches2);
+}
+
 #[test]
-fn parse_wild_samples() {
-    let samples_path = PathBuf::from(file!())
-        .parent()
-        .unwrap()
-        .join("wild-samples");
-    for file in fs::read_dir(samples_path).unwrap() {
-        let path = file.unwrap().path();
-        if path.extension().unwrap_or_default() != "patch" {
-            continue;
-        }
+fn parse_samples() {
+    visit_patches("samples", "diff", verify_patch_roundtrip);
+}
 
-        let data = fs::read_to_string(dbg!(&path)).unwrap();
-        let patches = Patch::from_multiple(&data)
-            .unwrap_or_else(|err| panic!("failed to parse {:?}, error: {}", path, err));
+#[test]
+fn parse_wild_samples() {
+    visit_patches("wild-samples", "patch", verify_patch_roundtrip);
+}
 
-        // Make sure that the patch file we produce parses to the same information as the original
-        // patch file.
-        #[allow(clippy::format_collect)] // Display::fmt is the only way to resolve Patch->str
-        let patch_file: String = patches.iter().map(|patch| format!("{}\n", patch)).collect();
+fn expect_parse_failure(data: &str, path: &PathBuf) {
+    println!("Expecting failure reading {:?}", path);
+    assert!(
+        Patch::from_multiple(data).is_err(),
+        "Expected failure parsing {:?}.  Has this been fixed?  \
+         Consider moving it to wild-samples for full regression test",
+        path
+    );
+}
 
-        let patches2 = Patch::from_multiple(&patch_file).unwrap_or_else(|err| {
-            panic!(
-                "failed to re-parse {:?} after formatting, error: {}",
-                path, err
-            )
-        });
-        assert_eq!(patches, patches2);
-    }
+// We have a zoo of known failing examples in wild-samples-fail.
+//
+// If we fix the underlying problem, this test will start to FAIL and patches
+// should be migrated out of the failing list to the wild-samples collection.
+#[test]
+fn parse_failing_wild_samples() {
+    visit_patches("wild-samples-fail", "patch", expect_parse_failure);
 }

tests/wild-samples-fail/.gitattributes

@@ -0,0 +1,2 @@
+# Disable text processing on these wild sample - eg preserve their wild CRLF format
+*.patch -text

tests/wild-samples-fail/merge-conflict.patch

@@ -0,0 +1,74 @@
+diff --cc src/parser.rs
+index cfc5da4,ac7ada2..0000000
+--- a/src/parser.rs
++++ b/src/parser.rs
+@@@ -104,15 -95,13 +104,22 @@@ fn multiple_patches(input: Input) -> IR
+      many1(patch)(input)
+  }
+
+++<<<<<<< HEAD
+ +fn patch(input: Input) -> IResult<Input, Patch> {
+ +    if let Ok(patch) = binary_files_differ(input) {
+ +        return Ok(patch);
+ +    }
+ +    if let Ok(patch) = file_rename_only(input) {
+ +        return Ok(patch);
+ +    }
+++=======
++ fn patch(input: Input<'_>) -> IResult<Input<'_>, Patch> {
++     if let Ok(patch) = binary_files_differ(input) {
++         return Ok(patch);
++     }
+++>>>>>>> 0b90edb (Handle "Binary files differ" lines)
+      let (input, files) = headers(input)?;
+      let (input, hunks) = chunks(input)?;
+ -    let (input, no_newline_indicator) = no_newline_indicator(input)?;
+      // Ignore trailing empty lines produced by some diff programs
+      let (input, _) = many0(line_ending)(input)?;
+
+@@@ -200,10 -117,43 +207,43 @@@ fn file_rename_only(input: Input<'_>) -
+      ))
+  }
+
++ /// Recognize a "binary files XX and YY differ" line as an empty patch.
++ fn binary_files_differ(input: Input<'_>) -> IResult<Input<'_>, Patch> {
++     // let (input, _) = context("Binary file line", tag("Binary files "))(input)?;
++     // The names aren't quoted so this seems to require lookahead and then
++     // parsing the identified string.
++     let (input, (old, new)) = context(
++         "Binary file line",
++         delimited(
++             tag("Binary files "),
++             map_opt(take_until(" differ\n"), |names: Input<'_>| {
++                 names.split_once(" and ")
++             }),
++             pair(tag(" differ"), line_ending),
++         ),
++     )(input)?;
++     dbg!(&old, &new);
++     Ok((
++         input,
++         Patch {
++             old: File {
++                 path: Cow::Borrowed(old),
++                 meta: None,
++             },
++             new: File {
++                 path: Cow::Borrowed(new),
++                 meta: None,
++             },
++             hunks: Vec::new(),
++             end_newline: false,
++         },
++     ))
++ }
++ 
+  // Header lines
+ -fn headers(input: Input<'_>) -> IResult<Input<'_>, (File, File)> {
+ +fn headers(input: Input) -> IResult<Input, (File, File)> {
+      // Ignore any preamble lines in produced diffs
+ -    let (input, _) = take_until("---")(input)?;
+ +    let (input, _) = take_until("--- ")(input)?;
+      let (input, _) = tag("--- ")(input)?;
+      let (input, oldfile) = header_line_content(input)?;
+      let (input, _) = line_ending(input)?;

tests/wild-samples-fail/submodule-log.patch

@@ -0,0 +1,5 @@
+Submodule submodule b0007f5..212f9bb:
+  < update README.md with Known Issues section
+  > add --verbose
+  > add --cached
+  > main: return error if we didn't pop

tests/wild-samples/sample2.patch

@@ -0,0 +1,53 @@
+# HG changeset patch
+# Parent 13ba6cbdb304cd251fbc22466cadb21019ee817f
+# User Bill McCloskey <wmccloskey@mozilla.com>
+
+diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp
+--- a/content/base/src/nsContentUtils.cpp
++++ b/content/base/src/nsContentUtils.cpp
+@@ -6369,17 +6369,17 @@ public:
+                                     nsCycleCollectionParticipant* helper)
+   {
+   }
+
+   NS_IMETHOD_(void) NoteNextEdgeName(const char* name)
+   {
+   }
+
+-  NS_IMETHOD_(void) NoteWeakMapping(void* map, void* key, void* val)
++  NS_IMETHOD_(void) NoteWeakMapping(void* map, void* key, void* kdelegate, void* val)
+   {
+   }
+
+   bool mFound;
+
+ private:
+   void* mWrapper;
+ };
+diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp
+--- a/js/src/jsfriendapi.cpp
++++ b/js/src/jsfriendapi.cpp
+@@ -527,16 +527,24 @@ js::VisitGrayWrapperTargets(JSCompartmen
+ {
+     for (WrapperMap::Enum e(comp->crossCompartmentWrappers); !e.empty(); e.popFront()) {
+         gc::Cell *thing = e.front().key.wrapped;
+         if (thing->isMarked(gc::GRAY))
+             callback(closure, thing);
+     }
+ }
+
++JS_FRIEND_API(JSObject *)
++js::GetWeakmapKeyDelegate(JSObject *key)
++{
++    if (JSWeakmapKeyDelegateOp op = key->getClass()->ext.weakmapKeyDelegateOp)
++        return op(key);
++    return NULL;
++}
++
+ JS_FRIEND_API(void)
+ JS_SetAccumulateTelemetryCallback(JSRuntime *rt, JSAccumulateTelemetryDataCallback callback)
+ {
+     rt->telemetryCallback = callback;
+ }
+
+ JS_FRIEND_API(JSObject *)

tests/wild-samples/submodule-hex.patch

@@ -0,0 +1,7 @@
+diff --git a/submodule b/submodule
+index 744c3350..e1e93dd3 160000
+--- a/submodule
++++ b/submodule
+@@ -1 +1 @@
+-Subproject commit 744c3350dd3ca95ac6ffadf294112d3b85f535d2
++Subproject commit e1e93dd3218a9ea2ac6300e61c09f0b282e7b5bc