Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
emilybache opened this issue Aug 10, 2022 · 6 comments · Fixed by #685
Closed

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

emilybache opened this issue Aug 10, 2022 · 6 comments · Fixed by #685
Milestone

Comments

@emilybache
Copy link

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:

  • the verified file is stored as .txt instead of .xml
  • if the xml comes formatted without newlines it is hard to read the diff when the test fails
  • if the xml comes in a slightly non-canonical form but is syntactically identical then the test will still fail

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
Copy link

lawrencek76 commented Aug 11, 2022

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
Copy link

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
Copy link
Author

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
Copy link
Member

@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
Copy link
Member

can u try v18.1-beta.1

@emilybache
Copy link
Author

Perfect! Thankyou so much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants