VerifyTests / Verify

Verify is a snapshot tool that simplifies the assertion of complex data models and documents.
MIT License
2.51k stars 141 forks source link

Support for verifying files in xml format, similar to existing support for json #585

Closed emilybache closed 2 years ago

emilybache commented 2 years ago

Is the feature request related to a problem

It's useful to be able to use Verify text that has a structure and syntax, and there is already good support for JSON. Xml is not used as much these days but I work with programmers who use it and where it would be useful to test with Verify. You can of course use Verify to check xml strings already, but it has a few limitations:

Describe the solution

It means adding a method 'VerifyXml' to the Verifier. I intend to look carefully at how 'VerifyJson' is implemented and do something very similar. I'd like to overload the 'VerifyXml' method with versions that take a string, and also System.XmlNode instances.

I have a code kata exercise that I maintain in several programming languages which outputs xml. I've implemented a 'VerifyXml' method there which shows what I'd like to be able to do:

https://github.com/emilybache/Product-Export-Refactoring-Kata/blob/with_tests/CSharp/ProductExport/XmlExporterTest.cs

Describe alternatives considered

I havn't yet considered much and am very open to suggestions from more experienced maintainers.

Additional context

I implemented this feature in Approvals a while back and although I'm not familiar with this code I hope it will be straightforward.

lawrencek76 commented 2 years ago

I have also just been looking at a similar issue but more in regard to namespaces in xml.

public static string xml1a = @"<?xml version=""1.0"" encoding=""utf-8"" ?><root xmlns=""namespace1"" ><child xmlns=""namespace2"" /></root>";
public static string xml1b = @"<?xml version=""1.0"" encoding=""utf-8"" ?><n1:root xmlns:n1=""namespace1"" xmlns:n2=""namespace2"" ><n2:child /></n1:root>";
public static string xml1c = @"<?xml version=""1.0"" encoding=""utf-8"" ?><n1:root xmlns:n1=""namespace1"" ><n2:child xmlns:n2=""namespace2""/></n1:root>";
public static string xml1d = @"<?xml version=""1.0"" encoding=""utf-8"" ?><n1:root xmlns=""namespace2"" xmlns:n1=""namespace1"" ><child /></n1:root>";

all of those are equivalent xml but parsing with XDocument.Parse() and then verifying fails.

My current thought is to fully normalize the namespaces by adding a namespace prefix to every element, move all namespace declarations to root in sorted order and avoiding the use of any default namespaces. if the xml did not contain any xmlns attributes this could be skipped.

lawrencek76 commented 2 years ago

Ok I ended up with this extension method to do the Normalization of the xdocument

        public static XDocument Normalize(this XDocument document)
        {
            var nsDict = new Dictionary<XNamespace, string>();
            document.Root!.DescendantsAndSelf().Attributes().
                Where(a => a.IsNamespaceDeclaration).OrderBy(a => a.Value).ToList().ForEach(a =>
                {
                    if (!nsDict.ContainsKey(a.Value))
                    {
                        nsDict.Add(a.Value, $"n{nsDict.Count + 1}");
                    }
                    a.Remove();
                });

            foreach (var ns in nsDict)
            {
                document.Root.Add(new XAttribute(XNamespace.Xmlns + $"{ns.Value}", ns.Key));
            }
            return document;
        }

Not sure if this is worth integrating into the library or not but maybe it helps someone else too.

emilybache commented 2 years ago

Now that I know a bit more about the support Verify already has for XDocuments I was able to re-write my VerifyXml code:

https://github.com/emilybache/Product-Export-Refactoring-Kata/blob/with_tests/CSharp/ProductExport/XmlExporterTest.cs

It's not very many lines of code. Would it be useful to have this VerifyXml method in Verifier? Or should I just continue to do it like this?

SimonCropp commented 2 years ago

@emilybache

I think what I understand from you is that you don't want to add a 'VerifyXml' to Verifier. Perhaps we could discuss that further in the other github issue.

after thinking about it. it think VerifyXml is a valid feature request. I will have a think about how to best implement it

SimonCropp commented 2 years ago

can u try v18.1-beta.1

emilybache commented 1 year ago

Perfect! Thankyou so much