Handle and test xmllint versions that return a zero exit code even if validation fails

[climbfuji]

Handle and test xmllint versions that return a zero exit code even if validation fails

1. In scripts/parse_tools/xml_tools.py, handle xmllint versions that return a zero exit code even if the validation fails. Simply logic, remove unused code, and update test/unit_tests/test_sdf.py accordingly
2. Create test/unit_tests/xmllint_wrapper/xmllint wrapper that emulates the behavior of "bad" xmllint which returns a zero exit code even if the validation fails, and test in GitHub actions

Notes.

1. This PR is based on code suggested by @gold2718 in a PR that got merged but later reverted due to problems with (unrelated) regular expression changes - see Add unit tests for SDF reading climbfuji/ccpp-framework#5. Credits to @gold2718 for this contribution.
2. call_command was only used in one place (validate XML files) and contained overly complicated logic to deal with optional arguments that were unnecessary. With the need to parse stdout/stderr from the subprocess in the validate_xml function, there would have been even more code duplication and unnecessary logic.
3. When Add capability to parse nested suites in capgen #691 was merged, xmllint became a hard requirement, therefore we can remove logic that allows silent failures of the XML validation step when xmllint cannot be found (silent failures in general are a bad idea imo).

User interface changes?: No

Fixes #700

Testing:
unit tests: pytest is now run in GitHub actions with both "good" and "bad" xmllint versions
manual testing: ran Python doctest on the updated xml_tools.py file, ran pytest locally on my laptop with "good" and "bad" xmllint

[climbfuji]

Unrelated bug fix uncovered while working on this PR

[mkavulich]

@climbfuji I'm a bit confused about how this new test helps the issue. I thought your intention was to run a test on a known bad XML, and if that returns a zero exit code, use that to determine that the xmllint version is bad? With this test it seems like we're just creating a test that will never fail with a non-zero exit code, unless I'm misunderstanding something.

[climbfuji]

@climbfuji I'm a bit confused about how this new test helps the issue. I thought your intention was to run a test on a known bad XML, and if that returns a zero exit code, use that to determine that the xmllint version is bad? With this test it seems like we're just creating a test that will never fail with a non-zero exit code, unless I'm misunderstanding something.

No, the intention is to create a fake xmllint version that behaves like the "bad" version that @gold2718 somehow ended up with and then make sure that this version can also be used and produces the correct results. The fake xmllint version is the newly added xmllint wrapper.

[peverwhee]

thanks @climbfuji !

[dustinswales]

These changes look good to me. Thanks!

[mkavulich]

@climbfuji I assume we want to wait for @gold2718 to review this one before merging?

[climbfuji]

@climbfuji I assume we want to wait for @gold2718 to review this one before merging?

Yes, this is only five days old. Thanks for checking.

[gold2718]

This looks good (i.e., way better than my version), thanks!