Skip to content

Commit 9aecf2f

Browse files
jan-cernyMab879
andauthored
Apply suggestions from code review
Co-authored-by: Matthew Burket <m@tthewburket.com>
1 parent 600b6d9 commit 9aecf2f

1 file changed

Lines changed: 16 additions & 16 deletions

File tree

_posts/2022-10-24-xmldiff-unit-tests.md

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ Recently, we have decided to improve the test coverage of the [ComplianceAsCode]
1111
Specifically, we have focused on testing code that works with XML.
1212
We have been creating tests for methods that generate XML elements or generate XML trees or transform one XML tree to another.
1313

14-
At the first sight, testing these types of methods looks easy.
15-
We have created some fixtures and then we have written some test cases with asserts counting the amount of generated elements and attributes and checking the expected values.
16-
That looked like in this example:
14+
At first sight, testing these types of methods looks easy.
15+
We created some fixtures and then wrote some test cases with asserts counting the amount of generated elements and attributes and checking the expected values.
16+
An example of this is below:
1717

1818
```python
1919
def test_group_to_xml_element(group_selinux):
@@ -26,12 +26,12 @@ def test_group_to_xml_element(group_selinux):
2626
... snip ...
2727
```
2828

29-
This is quite easy and most of the people would be fine with a test case like this.
29+
This is quite easy and most people would be fine with a test case like this.
3030
The advantage of this approach was that every requirement on the tested method had its own assert so when test started to fail it was immediately obvious what is broken.
3131
However, we didn't quite like it.
3232
The expected XML structure generated by the tested method (`to_xml_element()` in the example above) isn't clear from the code.
3333
The test can be quite long and it is laborious to write all the asserts for methods generating big XML trees with many child elements.
34-
So we have started to look for options of improving the tests.
34+
So we have started to look for options for improving the tests.
3535

3636
## Get familiar with xmldiff
3737

@@ -56,7 +56,7 @@ $ xmldiff file1.xml file2.xml
5656

5757
The `xmldiff` command will return a list of actions.
5858
This list of actions is so-called "Edit Script" and contains all changes needed to transform the first compared XML to the second compared XML.
59-
In the example above, we can see there are 2 differences between the 2 XML files.
59+
In the example above, we can see there are two differences between the two XML files.
6060
First is that the attribute `idref` on element described by XPath expression `/ns0:Rule/ns0:platform[1]` is changed to `virtual`.
6161
Second is that the text of the element described by XPath expression `/ns0:Rule/ns0:ident[1]` is changed to `777777`.
6262

@@ -68,7 +68,7 @@ diff = xmldiff.main.diff_files("file1.xml","file2.xml")
6868
print(diff)
6969
```
7070

71-
It seems that the `xmldiff` is very easy to be used so we have decided to use it in our unit tests.
71+
It seems that the `xmldiff` is very easy to use, so we have decided to use it in our unit tests.
7272
The [xmldiff documentation](https://xmldiff.readthedocs.io/en/stable/) is a good starting point.
7373

7474
But, we have encountered some small caveats, which we will describe below.
@@ -94,15 +94,15 @@ def test_group_to_xml_element(group_selinux, group_selinux_xml):
9494

9595
## Handling white space
9696

97-
However, then we reviewed our code and we kind of didn't like the saved XML test data -- they were ugly, with no nice formatting.
98-
So we naturally decided to apply `xmllint` pretty format and then the XMLs look pretty.
97+
However, then we reviewed our code and we didn't like the saved XML test data &mdash; they were ugly, with no nice formatting.
98+
So we decided to apply `xmllint` pretty format and then the XMLs look pretty.
9999
But, the tests started to fail.
100100

101-
We have found that the `xmldiff` is very sensitive produced a bunch of differences that we add newline and whitespace here and there.
101+
We have found that the `xmldiff` is very sensitive and produced a bunch of differences that we add newline and whitespace here and there.
102102
We were wondering how to convince `xmldiff` to ignore the whitespace.
103-
We didn't want to run `xmllint` command as a sub process in our tests.
103+
We didn't want to run `xmllint` command as a subprocess in our tests.
104104
We tried to use [formatters](https://xmldiff.readthedocs.io/en/stable/api.html#using-formatters) but with no luck, xmllint still behaved sensitively to whitespace.
105-
We were mainly concerned that the data in the form they are stored are will be difficult to review and the whitespace sensitivity will make them cumbersome to maintain.
105+
We were mainly concerned that the data in the stored form would be difficult to review and the whitespace sensitivity would make them cumbersome to maintain.
106106
By accident, we have discovered that this behavior doesn't happen with the `xmllint.main.diff_files()` method.
107107
That method isn't sensitive to whitespace or formatting of the XML files, so we can save them in a pretty format.
108108
So we reworked our tests so that the test first saved the output of the tested method to a temporary file and then we called `xmllint.main.diff_files()` to compare this temporary file with our static file in test data.
@@ -123,8 +123,8 @@ Note: The `temporary_filename` is a context manager that gives us a temporary fi
123123

124124
## Working with namespaces
125125

126-
One of our methods transforms a given XML tree to a different XML tree that differs in a couple of attributes and values but rest of the tree is the same.
127-
So we have compared the input of this method with the output of this method using `xmldiff` and we got the diff in a form of an Edit script.
126+
One of our methods transforms a given XML tree to a different XML tree that differs in a couple of attributes and values but the rest of the tree is the same.
127+
So we have compared the input of this method with the output of this method using `xmldiff` and we got the diff in the form of an Edit script.
128128
Then, we had to solve how to write an assert that this Edit script is the expected one.
129129
In other words, to verify that the `xmldiff` has given the expected diff.
130130
We found that the items in the diff are Python `namedtuple`s and that we can easily create our own `namedtuple`s in the code and then check if they're present in the diff.
@@ -162,9 +162,9 @@ def test_foo(old, new):
162162

163163
Another problem that we faced is that we wanted to use the `xmldiff` tests in our upstream and downstream CI.
164164
Unfortunately, we discovered that the library isn't available as RPM, neither in Fedora nor in RHEL.
165-
It's available only in PyPi.
165+
It's available only in PyPI.
166166
That means we can't execute the tests in some of our test environments.
167-
But, we wanted to still run the tests in the environments where `xmldiff` is available and at the same time not disable all the unit tests on the other systems. Fortunately, `pytest` has a very elegant method `importorskip()` that makes the test case be skipped when some module isn't available and still runs the other test cases.
167+
But, we wanted to still run the tests in the environments where `xmldiff` is available and at the same time not disable all the unit tests on the other systems. Fortunately, `pytest` has a very elegant method `importorskip()` that skips the test case when some module isn't available and still runs the other test cases.
168168

169169
We have used this method in every test function where we use `xmldiff`:
170170

0 commit comments

Comments
 (0)