From 1c5551a917febf7b97b0bd7dc470d7235d4bd10b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maxime=20=E2=80=9Cpep=E2=80=9D=20Buquet?= <pep@bouah.net>
Date: Fri, 29 Nov 2019 14:33:17 +0100
Subject: [PATCH 1/5] minidom: Implement PartialEq manually for Node and
 Element
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Move the NamespaceAwareCompare implementation from xmpp-parsers as Node
and Element's PartialEq implementation. Thanks Astro!

It's a lot more useful in tests to use `assert_eq!` than `assert!`, so
we get both items compared (left and right) instead of a "it failed."
message.

This "breaks" comparison for these two structs in the sense that it is
not strict object comparison anymore but it ensures that namespaces are
all present in the compared objects.

Signed-off-by: Maxime “pep” Buquet <pep@bouah.net>
---
 minidom-rs/CHANGELOG.md                   |   4 +-
 minidom-rs/src/element.rs                 |  27 ++++-
 minidom-rs/src/node.rs                    |  12 ++-
 xmpp-parsers/src/bookmarks.rs             |   3 +-
 xmpp-parsers/src/bookmarks2.rs            |   3 +-
 xmpp-parsers/src/disco.rs                 |   3 +-
 xmpp-parsers/src/ibr.rs                   |   5 +-
 xmpp-parsers/src/iq.rs                    |   7 +-
 xmpp-parsers/src/jingle_s5b.rs            |   5 +-
 xmpp-parsers/src/message.rs               |   7 +-
 xmpp-parsers/src/muc/muc.rs               |   3 +-
 xmpp-parsers/src/muc/user.rs              |   3 +-
 xmpp-parsers/src/presence.rs              |   3 +-
 xmpp-parsers/src/pubsub/event.rs          |   3 +-
 xmpp-parsers/src/pubsub/pubsub.rs         |  11 +-
 xmpp-parsers/src/roster.rs                |   3 +-
 xmpp-parsers/src/rsm.rs                   |   3 +-
 xmpp-parsers/src/util/compare_elements.rs | 125 ----------------------
 xmpp-parsers/src/util/mod.rs              |   4 -
 xmpp-parsers/src/version.rs               |   3 +-
 20 files changed, 65 insertions(+), 172 deletions(-)
 delete mode 100644 xmpp-parsers/src/util/compare_elements.rs

diff --git a/minidom-rs/CHANGELOG.md b/minidom-rs/CHANGELOG.md
index 9cef50c..6564977 100644
--- a/minidom-rs/CHANGELOG.md
+++ b/minidom-rs/CHANGELOG.md
@@ -1,6 +1,8 @@
 Version XXX, released YYY:
   * Breaking
-    `Element.write_to` doesn't prepand xml prelude anymore. Use `write_to_decl`.
+    * `Element.write_to` doesn't prepand xml prelude anymore. Use `write_to_decl`.
+    * PartialEq implementation for Element and Node have been changed to
+      ensure namespaces match even if the objects are slightly different.
   * Changes
     * Update edition to 2018
     * Add NSChoice enum to allow comparing NSs differently
diff --git a/minidom-rs/src/element.rs b/minidom-rs/src/element.rs
index 9907bc5..9d04065 100644
--- a/minidom-rs/src/element.rs
+++ b/minidom-rs/src/element.rs
@@ -68,7 +68,7 @@ pub fn escape(raw: &[u8]) -> Cow<[u8]> {
     }
 }
 
-#[derive(Clone, PartialEq, Eq, Debug)]
+#[derive(Clone, Eq, Debug)]
 /// A struct representing a DOM Element.
 pub struct Element {
     prefix: Option<String>,
@@ -95,6 +95,31 @@ impl FromStr for Element {
     }
 }
 
+impl PartialEq for Element {
+    fn eq(&self, other: &Self) -> bool {
+        if self.name() == other.name() && self.ns() == other.ns() && self.attrs().eq(other.attrs())
+        {
+            let child_elems = self.children().count();
+            let text_is_whitespace = self
+                .texts()
+                .all(|text| text.chars().all(char::is_whitespace));
+            if child_elems > 0 && text_is_whitespace {
+                // Ignore all the whitespace text nodes
+                self.children()
+                    .zip(other.children())
+                    .all(|(node1, node2)| node1 == node2)
+            } else {
+                // Compare with text nodes
+                self.nodes()
+                    .zip(other.nodes())
+                    .all(|(node1, node2)| node1 == node2)
+            }
+        } else {
+            false
+        }
+    }
+}
+
 impl Element {
     fn new<NS: Into<NamespaceSet>>(
         name: String,
diff --git a/minidom-rs/src/node.rs b/minidom-rs/src/node.rs
index 33f3ad0..5f67502 100644
--- a/minidom-rs/src/node.rs
+++ b/minidom-rs/src/node.rs
@@ -9,7 +9,7 @@ use quick_xml::events::{BytesText, Event};
 use quick_xml::Writer as EventWriter;
 
 /// A node in an element tree.
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, Eq)]
 pub enum Node {
     /// An `Element`.
     Element(Element),
@@ -208,3 +208,13 @@ impl From<ElementBuilder> for Node {
         Node::Element(builder.build())
     }
 }
+
+impl PartialEq for Node {
+    fn eq(&self, other: &Self) -> bool {
+        match (self, other) {
+            (&Node::Element(ref elem1), &Node::Element(ref elem2)) => elem1 == elem2,
+            (&Node::Text(ref text1), &Node::Text(ref text2)) => text1 == text2,
+            _ => false,
+        }
+    }
+}
diff --git a/xmpp-parsers/src/bookmarks.rs b/xmpp-parsers/src/bookmarks.rs
index 900a3a9..7346cd8 100644
--- a/xmpp-parsers/src/bookmarks.rs
+++ b/xmpp-parsers/src/bookmarks.rs
@@ -70,7 +70,6 @@ impl Storage {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use crate::Element;
     use std::convert::TryFrom;
 
@@ -99,7 +98,7 @@ mod tests {
         assert_eq!(storage.urls.len(), 0);
 
         let elem2 = Element::from(Storage::new());
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
diff --git a/xmpp-parsers/src/bookmarks2.rs b/xmpp-parsers/src/bookmarks2.rs
index cf3eaf6..ff84be9 100644
--- a/xmpp-parsers/src/bookmarks2.rs
+++ b/xmpp-parsers/src/bookmarks2.rs
@@ -48,7 +48,6 @@ mod tests {
     use crate::ns;
     use crate::pubsub::event::PubSubEvent;
     use crate::pubsub::pubsub::Item as PubSubItem;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use crate::Element;
     use std::convert::TryFrom;
 
@@ -77,7 +76,7 @@ mod tests {
         assert_eq!(conference.password, None);
 
         let elem2 = Element::from(Conference::new());
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
diff --git a/xmpp-parsers/src/disco.rs b/xmpp-parsers/src/disco.rs
index 2041819..6621f8b 100644
--- a/xmpp-parsers/src/disco.rs
+++ b/xmpp-parsers/src/disco.rs
@@ -232,7 +232,6 @@ impl IqResultPayload for DiscoItemsResult {}
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use std::str::FromStr;
 
     #[cfg(target_pointer_width = "32")]
@@ -301,7 +300,7 @@ mod tests {
         assert_eq!(query.extensions[0].form_type, Some(String::from("example")));
 
         let elem2 = query.into();
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
diff --git a/xmpp-parsers/src/ibr.rs b/xmpp-parsers/src/ibr.rs
index 72f582f..31d0e33 100644
--- a/xmpp-parsers/src/ibr.rs
+++ b/xmpp-parsers/src/ibr.rs
@@ -117,7 +117,6 @@ impl From<Query> for Element {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
 
     #[cfg(target_pointer_width = "32")]
     #[test]
@@ -207,7 +206,7 @@ mod tests {
         let form = query.form.clone().unwrap();
         assert!(!form.instructions.unwrap().is_empty());
         let elem2 = query.into();
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
@@ -242,6 +241,6 @@ mod tests {
             panic!();
         }
         let elem2 = query.into();
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 }
diff --git a/xmpp-parsers/src/iq.rs b/xmpp-parsers/src/iq.rs
index 0553528..0f6165d 100644
--- a/xmpp-parsers/src/iq.rs
+++ b/xmpp-parsers/src/iq.rs
@@ -220,7 +220,6 @@ mod tests {
     use super::*;
     use crate::disco::DiscoInfoQuery;
     use crate::stanza_error::{DefinedCondition, ErrorType};
-    use crate::util::compare_elements::NamespaceAwareCompare;
 
     #[cfg(target_pointer_width = "32")]
     #[test]
@@ -283,7 +282,7 @@ mod tests {
         assert_eq!(iq.to, None);
         assert_eq!(&iq.id, "foo");
         assert!(match iq.payload {
-            IqType::Get(element) => element.compare_to(&query),
+            IqType::Get(element) => element == query,
             _ => false,
         });
     }
@@ -308,7 +307,7 @@ mod tests {
         assert_eq!(iq.to, None);
         assert_eq!(&iq.id, "vcard");
         assert!(match iq.payload {
-            IqType::Set(element) => element.compare_to(&vcard),
+            IqType::Set(element) => element == vcard,
             _ => false,
         });
     }
@@ -355,7 +354,7 @@ mod tests {
         assert_eq!(iq.to, None);
         assert_eq!(&iq.id, "res");
         assert!(match iq.payload {
-            IqType::Result(Some(element)) => element.compare_to(&query),
+            IqType::Result(Some(element)) => element == query,
             _ => false,
         });
     }
diff --git a/xmpp-parsers/src/jingle_s5b.rs b/xmpp-parsers/src/jingle_s5b.rs
index 2d72080..816da52 100644
--- a/xmpp-parsers/src/jingle_s5b.rs
+++ b/xmpp-parsers/src/jingle_s5b.rs
@@ -275,7 +275,6 @@ impl From<Transport> for Element {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use std::str::FromStr;
 
     #[cfg(target_pointer_width = "32")]
@@ -327,7 +326,7 @@ mod tests {
             payload: TransportPayload::Activated(CandidateId(String::from("coucou"))),
         };
         let elem2: Element = transport.into();
-        assert!(elem.compare_to(&elem2));
+        assert_eq!(elem, elem2);
     }
 
     #[test]
@@ -347,6 +346,6 @@ mod tests {
             }]),
         };
         let elem2: Element = transport.into();
-        assert!(elem.compare_to(&elem2));
+        assert_eq!(elem, elem2);
     }
 }
diff --git a/xmpp-parsers/src/message.rs b/xmpp-parsers/src/message.rs
index 4c4ce26..4e6cb3b 100644
--- a/xmpp-parsers/src/message.rs
+++ b/xmpp-parsers/src/message.rs
@@ -242,7 +242,6 @@ impl From<Message> for Element {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use std::str::FromStr;
 
     #[cfg(target_pointer_width = "32")]
@@ -312,7 +311,7 @@ mod tests {
         }
 
         let elem2 = message.into();
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
@@ -326,7 +325,7 @@ mod tests {
             .bodies
             .insert(String::from(""), Body::from_str("Hello world!").unwrap());
         let elem2 = message.into();
-        assert!(elem.compare_to(&elem2));
+        assert_eq!(elem, elem2);
     }
 
     #[test]
@@ -349,7 +348,7 @@ mod tests {
         }
 
         let elem2 = message.into();
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
diff --git a/xmpp-parsers/src/muc/muc.rs b/xmpp-parsers/src/muc/muc.rs
index 935edd5..ec0954c 100644
--- a/xmpp-parsers/src/muc/muc.rs
+++ b/xmpp-parsers/src/muc/muc.rs
@@ -94,7 +94,6 @@ impl Muc {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use crate::util::error::Error;
     use crate::Element;
     use std::convert::TryFrom;
@@ -161,7 +160,7 @@ mod tests {
         assert_eq!(muc.password, Some("coucou".to_owned()));
 
         let elem2 = Element::from(muc);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
diff --git a/xmpp-parsers/src/muc/user.rs b/xmpp-parsers/src/muc/user.rs
index b90298e..bcb997e 100644
--- a/xmpp-parsers/src/muc/user.rs
+++ b/xmpp-parsers/src/muc/user.rs
@@ -236,7 +236,6 @@ generate_element!(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use std::error::Error as StdError;
 
     #[test]
@@ -298,7 +297,7 @@ mod tests {
             items: vec![],
         };
         let elem2 = muc.into();
-        assert!(elem.compare_to(&elem2));
+        assert_eq!(elem, elem2);
     }
 
     #[cfg(not(feature = "disable-validation"))]
diff --git a/xmpp-parsers/src/presence.rs b/xmpp-parsers/src/presence.rs
index 082eefb..8a67dde 100644
--- a/xmpp-parsers/src/presence.rs
+++ b/xmpp-parsers/src/presence.rs
@@ -335,7 +335,6 @@ impl From<Presence> for Element {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use jid::{BareJid, FullJid};
 
     #[cfg(target_pointer_width = "32")]
@@ -382,7 +381,7 @@ mod tests {
             .unwrap();
         let presence = Presence::new(Type::Unavailable);
         let elem2 = presence.into();
-        assert!(elem.compare_to(&elem2));
+        assert_eq!(elem, elem2);
     }
 
     #[test]
diff --git a/xmpp-parsers/src/pubsub/event.rs b/xmpp-parsers/src/pubsub/event.rs
index 322a79f..d46b750 100644
--- a/xmpp-parsers/src/pubsub/event.rs
+++ b/xmpp-parsers/src/pubsub/event.rs
@@ -246,7 +246,6 @@ impl From<PubSubEvent> for Element {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use std::str::FromStr;
 
     #[test]
@@ -420,6 +419,6 @@ mod tests {
         }
 
         let elem2: Element = event.into();
-        assert!(elem.compare_to(&elem2));
+        assert_eq!(elem, elem2);
     }
 }
diff --git a/xmpp-parsers/src/pubsub/pubsub.rs b/xmpp-parsers/src/pubsub/pubsub.rs
index 24ad3c4..42e9bda 100644
--- a/xmpp-parsers/src/pubsub/pubsub.rs
+++ b/xmpp-parsers/src/pubsub/pubsub.rs
@@ -518,7 +518,6 @@ impl From<PubSub> for Element {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
 
     #[test]
     fn create() {
@@ -536,7 +535,7 @@ mod tests {
         }
 
         let elem2 = Element::from(pubsub);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
 
         let elem: Element =
             "<pubsub xmlns='http://jabber.org/protocol/pubsub'><create node='coucou'/></pubsub>"
@@ -553,7 +552,7 @@ mod tests {
         }
 
         let elem2 = Element::from(pubsub);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
@@ -573,7 +572,7 @@ mod tests {
         }
 
         let elem2 = Element::from(pubsub);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
@@ -596,7 +595,7 @@ mod tests {
         }
 
         let elem2 = Element::from(pubsub);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
@@ -616,7 +615,7 @@ mod tests {
         }
 
         let elem2 = Element::from(pubsub);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
diff --git a/xmpp-parsers/src/roster.rs b/xmpp-parsers/src/roster.rs
index 3153e46..ea33995 100644
--- a/xmpp-parsers/src/roster.rs
+++ b/xmpp-parsers/src/roster.rs
@@ -92,7 +92,6 @@ impl IqResultPayload for Roster {}
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use crate::util::error::Error;
     use crate::Element;
     use std::convert::TryFrom;
@@ -220,7 +219,7 @@ mod tests {
         assert_eq!(roster.items[0].groups[0], Group::from_str("A").unwrap());
         assert_eq!(roster.items[0].groups[1], Group::from_str("B").unwrap());
         let elem2 = roster.into();
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
diff --git a/xmpp-parsers/src/rsm.rs b/xmpp-parsers/src/rsm.rs
index f59235b..4e31b27 100644
--- a/xmpp-parsers/src/rsm.rs
+++ b/xmpp-parsers/src/rsm.rs
@@ -174,7 +174,6 @@ impl From<SetResult> for Element {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
 
     #[cfg(target_pointer_width = "32")]
     #[test]
@@ -304,6 +303,6 @@ mod tests {
             count: None,
         };
         let elem2 = set2.into();
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 }
diff --git a/xmpp-parsers/src/util/compare_elements.rs b/xmpp-parsers/src/util/compare_elements.rs
deleted file mode 100644
index 7494be7..0000000
--- a/xmpp-parsers/src/util/compare_elements.rs
+++ /dev/null
@@ -1,125 +0,0 @@
-// Copyright (c) 2017 Astro <astro@spaceboyz.net>
-//
-// This Source Code Form is subject to the terms of the Mozilla Public
-// License, v. 2.0. If a copy of the MPL was not distributed with this
-// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-use minidom::{Element, Node};
-
-pub trait NamespaceAwareCompare {
-    /// Namespace-aware comparison for tests
-    fn compare_to(&self, other: &Self) -> bool;
-}
-
-impl NamespaceAwareCompare for Node {
-    fn compare_to(&self, other: &Self) -> bool {
-        match (self, other) {
-            (&Node::Element(ref elem1), &Node::Element(ref elem2)) => {
-                Element::compare_to(elem1, elem2)
-            }
-            (&Node::Text(ref text1), &Node::Text(ref text2)) => text1 == text2,
-            _ => false,
-        }
-    }
-}
-
-impl NamespaceAwareCompare for Element {
-    fn compare_to(&self, other: &Self) -> bool {
-        if self.name() == other.name() && self.ns() == other.ns() && self.attrs().eq(other.attrs())
-        {
-            let child_elems = self.children().count();
-            let text_is_whitespace = self
-                .texts()
-                .all(|text| text.chars().all(char::is_whitespace));
-            if child_elems > 0 && text_is_whitespace {
-                // Ignore all the whitespace text nodes
-                self.children()
-                    .zip(other.children())
-                    .all(|(node1, node2)| node1.compare_to(node2))
-            } else {
-                // Compare with text nodes
-                self.nodes()
-                    .zip(other.nodes())
-                    .all(|(node1, node2)| node1.compare_to(node2))
-            }
-        } else {
-            false
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::Element;
-
-    #[test]
-    fn simple() {
-        let elem1: Element = "<a a='b'>x <l/> 3</a>".parse().unwrap();
-        let elem2: Element = "<a a='b'>x <l/> 3</a>".parse().unwrap();
-        assert!(elem1.compare_to(&elem2));
-    }
-
-    #[test]
-    fn wrong_attr_name() {
-        let elem1: Element = "<a a='b'>x 3</a>".parse().unwrap();
-        let elem2: Element = "<a c='b'>x 3</a>".parse().unwrap();
-        assert!(!elem1.compare_to(&elem2));
-    }
-
-    #[test]
-    fn wrong_attr_value() {
-        let elem1: Element = "<a a='b'>x 3</a>".parse().unwrap();
-        let elem2: Element = "<a a='c'>x 3</a>".parse().unwrap();
-        assert!(!elem1.compare_to(&elem2));
-    }
-
-    #[test]
-    fn attr_order() {
-        let elem1: Element = "<e1 a='b' c='d'/>".parse().unwrap();
-        let elem2: Element = "<e1 c='d' a='b'/>".parse().unwrap();
-        assert!(elem1.compare_to(&elem2));
-    }
-
-    #[test]
-    fn wrong_texts() {
-        let elem1: Element = "<e1>foo</e1>".parse().unwrap();
-        let elem2: Element = "<e1>bar</e1>".parse().unwrap();
-        assert!(!elem1.compare_to(&elem2));
-    }
-
-    #[test]
-    fn children() {
-        let elem1: Element = "<e1><foo/><bar/></e1>".parse().unwrap();
-        let elem2: Element = "<e1><foo/><bar/></e1>".parse().unwrap();
-        assert!(elem1.compare_to(&elem2));
-    }
-
-    #[test]
-    fn wrong_children() {
-        let elem1: Element = "<e1><foo/></e1>".parse().unwrap();
-        let elem2: Element = "<e1><bar/></e1>".parse().unwrap();
-        assert!(!elem1.compare_to(&elem2));
-    }
-
-    #[test]
-    fn xmlns_wrong() {
-        let elem1: Element = "<e1 xmlns='ns1'><foo/></e1>".parse().unwrap();
-        let elem2: Element = "<e1 xmlns='ns2'><foo/></e1>".parse().unwrap();
-        assert!(!elem1.compare_to(&elem2));
-    }
-
-    #[test]
-    fn xmlns_other_prefix() {
-        let elem1: Element = "<e1 xmlns='ns1'><foo/></e1>".parse().unwrap();
-        let elem2: Element = "<x:e1 xmlns:x='ns1'><x:foo/></x:e1>".parse().unwrap();
-        assert!(elem1.compare_to(&elem2));
-    }
-
-    #[test]
-    fn xmlns_dup() {
-        let elem1: Element = "<e1 xmlns='ns1'><foo/></e1>".parse().unwrap();
-        let elem2: Element = "<e1 xmlns='ns1'><foo  xmlns='ns1'/></e1>".parse().unwrap();
-        assert!(elem1.compare_to(&elem2));
-    }
-}
diff --git a/xmpp-parsers/src/util/mod.rs b/xmpp-parsers/src/util/mod.rs
index 2a595f9..1d3ee64 100644
--- a/xmpp-parsers/src/util/mod.rs
+++ b/xmpp-parsers/src/util/mod.rs
@@ -13,7 +13,3 @@ pub(crate) mod helpers;
 /// Helper macros to parse and serialise more easily.
 #[macro_use]
 mod macros;
-
-#[cfg(test)]
-/// Namespace-aware comparison for tests
-pub(crate) mod compare_elements;
diff --git a/xmpp-parsers/src/version.rs b/xmpp-parsers/src/version.rs
index 028ce96..2bd894b 100644
--- a/xmpp-parsers/src/version.rs
+++ b/xmpp-parsers/src/version.rs
@@ -41,7 +41,6 @@ impl IqResultPayload for VersionResult {}
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
     use crate::Element;
     use std::convert::TryFrom;
 
@@ -84,6 +83,6 @@ mod tests {
                 .parse()
                 .unwrap();
         println!("{:?}", elem1);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 }

From 8d41959497c2d9ca5683925af5f1f1cab4d54c41 Mon Sep 17 00:00:00 2001
From: Paul Fariello <paul.fariello@synacktiv.com>
Date: Wed, 27 Nov 2019 18:16:33 +0100
Subject: [PATCH 2/5] jingle: Add test on serialisation

---
 xmpp-parsers/src/jingle.rs | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/xmpp-parsers/src/jingle.rs b/xmpp-parsers/src/jingle.rs
index 8d64a22..5921d88 100644
--- a/xmpp-parsers/src/jingle.rs
+++ b/xmpp-parsers/src/jingle.rs
@@ -858,4 +858,39 @@ mod tests {
         };
         assert_eq!(message, "Text element present twice for the same xml:lang.");
     }
+
+    #[test]
+    fn test_serialize_jingle() {
+        let reference: Element = "<jingle xmlns='urn:xmpp:jingle:1' action='session-initiate' sid='a73sjjvkla37jfea'><content xmlns='urn:xmpp:jingle:1' creator='initiator' name='this-is-a-stub'><description xmlns='urn:xmpp:jingle:apps:stub:0'/><transport xmlns='urn:xmpp:jingle:transports:stub:0'/></content></jingle>"
+        .parse()
+        .unwrap();
+
+        let jingle = Jingle {
+            action: Action::SessionInitiate,
+            initiator: None,
+            responder: None,
+            sid: SessionId(String::from("a73sjjvkla37jfea")),
+            contents: vec![Content {
+                creator: Creator::Initiator,
+                disposition: Disposition::default(),
+                name: ContentId(String::from("this-is-a-stub")),
+                senders: Senders::default(),
+                description: Some(Description::Unknown(
+                    Element::builder("description")
+                        .ns("urn:xmpp:jingle:apps:stub:0")
+                        .build(),
+                )),
+                transport: Some(Transport::Unknown(
+                    Element::builder("transport")
+                        .ns("urn:xmpp:jingle:transports:stub:0")
+                        .build(),
+                )),
+                security: None,
+            }],
+            reason: None,
+            other: vec![],
+        };
+        let serialized: Element = jingle.into();
+        assert_eq!(serialized, reference);
+    }
 }

From 3a2027411ec798b509b16b2735a07b2b1622e789 Mon Sep 17 00:00:00 2001
From: Paul Fariello <paul.fariello@synacktiv.com>
Date: Wed, 27 Nov 2019 18:16:51 +0100
Subject: [PATCH 3/5] mam: Fix test on serialisation

---
 xmpp-parsers/src/mam.rs | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/xmpp-parsers/src/mam.rs b/xmpp-parsers/src/mam.rs
index d1a67a3..a644cef 100644
--- a/xmpp-parsers/src/mam.rs
+++ b/xmpp-parsers/src/mam.rs
@@ -391,21 +391,9 @@ mod tests {
         assert_eq!(elem, elem2);
     }
 
-    #[ignore]
     #[test]
     fn test_serialize_query_with() {
-        let elem: Element = r#"
-        <query xmlns='urn:xmpp:mam:2'>
-            <x xmlns='jabber:x:data' type='submit'>
-                <field var='FORM_TYPE' type='hidden'>
-                    <value>urn:xmpp:mam:2</value>
-                </field>
-                <field var='with'>
-                    <value>juliet@capulet.lit</value>
-                </field>
-            </x>
-        </query>
-        "#
+        let reference: Element = "<query xmlns='urn:xmpp:mam:2'><x xmlns='jabber:x:data' type='submit'><field xmlns='jabber:x:data' var='FORM_TYPE' type='hidden'><value xmlns='jabber:x:data'>urn:xmpp:mam:2</value></field><field xmlns='jabber:x:data' var='with'><value xmlns='jabber:x:data'>juliet@capulet.lit</value></field></x></query>"
         .parse()
         .unwrap();
 
@@ -415,10 +403,10 @@ mod tests {
             title: None,
             instructions: None,
             fields: vec![Field {
-                var: String::from("var"),
+                var: String::from("with"),
                 type_: FieldType::TextSingle,
                 label: None,
-                required: true,
+                required: false,
                 options: vec![],
                 values: vec![String::from("juliet@capulet.lit")],
                 media: vec![],
@@ -430,7 +418,7 @@ mod tests {
             set: None,
             form: Some(form),
         };
-        let elem2 = foo.into();
-        assert_eq!(elem, elem2);
+        let serialized: Element = foo.into();
+        assert_eq!(serialized, reference);
     }
 }

From 689b7cf8363f6b0ac2d67d2c3620b16afb8aa510 Mon Sep 17 00:00:00 2001
From: Paul Fariello <paul.fariello@synacktiv.com>
Date: Wed, 27 Nov 2019 18:17:10 +0100
Subject: [PATCH 4/5] Fix serialisation of Optional field

---
 xmpp-parsers/src/util/macros.rs | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/xmpp-parsers/src/util/macros.rs b/xmpp-parsers/src/util/macros.rs
index 1072fb1..52f3305 100644
--- a/xmpp-parsers/src/util/macros.rs
+++ b/xmpp-parsers/src/util/macros.rs
@@ -590,18 +590,18 @@ macro_rules! generate_serialiser {
         }))
     };
     ($builder:ident, $parent:ident, $elem:ident, Option, $constructor:ident, ($name:tt, *)) => {
-        $builder.append_all($parent.$elem.map(|elem| {
-            crate::Element::builder($name)
-                .ns(elem.get_ns())
-                .append(::minidom::Node::Element(crate::Element::from(elem)))
-        }))
+        $builder.append_all(
+            $parent
+                .$elem
+                .map(|elem| ::minidom::Node::Element(crate::Element::from(elem))),
+        )
     };
     ($builder:ident, $parent:ident, $elem:ident, Option, $constructor:ident, ($name:tt, $ns:ident)) => {
-        $builder.append_all($parent.$elem.map(|elem| {
-            crate::Element::builder($name)
-                .ns(crate::ns::$ns)
-                .append(::minidom::Node::Element(crate::Element::from(elem)))
-        }))
+        $builder.append_all(
+            $parent
+                .$elem
+                .map(|elem| ::minidom::Node::Element(crate::Element::from(elem))),
+        )
     };
     ($builder:ident, $parent:ident, $elem:ident, Vec, $constructor:ident, ($name:tt, $ns:ident)) => {
         $builder.append_all($parent.$elem.into_iter())

From 3c92f849d3730d4befd5bb1589d4f5f20d6d5c0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maxime=20=E2=80=9Cpep=E2=80=9D=20Buquet?= <pep@bouah.net>
Date: Sun, 1 Dec 2019 03:05:45 +0100
Subject: [PATCH 5/5] pubsub: Add serialization test for configure element
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Maxime “pep” Buquet <pep@bouah.net>
---
 xmpp-parsers/src/ns.rs            |  2 ++
 xmpp-parsers/src/pubsub/pubsub.rs | 38 ++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/xmpp-parsers/src/ns.rs b/xmpp-parsers/src/ns.rs
index 231d78e..511761a 100644
--- a/xmpp-parsers/src/ns.rs
+++ b/xmpp-parsers/src/ns.rs
@@ -52,6 +52,8 @@ pub const PUBSUB_ERRORS: &str = "http://jabber.org/protocol/pubsub#errors";
 pub const PUBSUB_EVENT: &str = "http://jabber.org/protocol/pubsub#event";
 /// XEP-0060: Publish-Subscribe
 pub const PUBSUB_OWNER: &str = "http://jabber.org/protocol/pubsub#owner";
+/// XEP-0060: Publish-Subscribe node configuration
+pub const PUBSUB_CONFIGURE: &str = "http://jabber.org/protocol/pubsub#node_config";
 
 /// XEP-0071: XHTML-IM
 pub const XHTML_IM: &str = "http://jabber.org/protocol/xhtml-im";
diff --git a/xmpp-parsers/src/pubsub/pubsub.rs b/xmpp-parsers/src/pubsub/pubsub.rs
index 42e9bda..13184f6 100644
--- a/xmpp-parsers/src/pubsub/pubsub.rs
+++ b/xmpp-parsers/src/pubsub/pubsub.rs
@@ -518,6 +518,7 @@ impl From<PubSub> for Element {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::data_forms::{DataForm, DataFormType, Field, FieldType};
 
     #[test]
     fn create() {
@@ -556,7 +557,7 @@ mod tests {
     }
 
     #[test]
-    fn create_and_configure() {
+    fn create_and_configure_empty() {
         let elem: Element =
             "<pubsub xmlns='http://jabber.org/protocol/pubsub'><create/><configure/></pubsub>"
                 .parse()
@@ -575,6 +576,41 @@ mod tests {
         assert_eq!(elem1, elem2);
     }
 
+    #[test]
+    fn create_and_configure_simple() {
+        // XXX: Do we want xmpp-parsers to always specify the field type in the output Element?
+        let elem: Element = "<pubsub xmlns='http://jabber.org/protocol/pubsub'><create node='foo'/><configure><x xmlns='jabber:x:data' type='submit'><field var='FORM_TYPE' type='hidden'><value>http://jabber.org/protocol/pubsub#node_config</value></field><field var='pubsub#access_model' type='list-single'><value>whitelist</value></field></x></configure></pubsub>"
+        .parse()
+        .unwrap();
+        let elem1 = elem.clone();
+
+        let pubsub = PubSub::Create {
+            create: Create {
+                node: Some(NodeName(String::from("foo"))),
+            },
+            configure: Some(Configure {
+                form: Some(DataForm {
+                    type_: DataFormType::Submit,
+                    form_type: Some(String::from(ns::PUBSUB_CONFIGURE)),
+                    title: None,
+                    instructions: None,
+                    fields: vec![Field {
+                        var: String::from("pubsub#access_model"),
+                        type_: FieldType::ListSingle,
+                        label: None,
+                        required: false,
+                        options: vec![],
+                        values: vec![String::from("whitelist")],
+                        media: vec![],
+                    }],
+                }),
+            }),
+        };
+
+        let elem2 = Element::from(pubsub);
+        assert_eq!(elem1, elem2);
+    }
+
     #[test]
     fn publish() {
         let elem: Element =