On Wed, 24 Jun 2026 at 18:03, Branko Čibej <[email protected]> wrote:
> On 24. 6. 2026 16:03, Ivan Zhakov wrote: > > On Tue, 23 Jun 2026 at 17:42, Daniel Sahlberg <[email protected]> > wrote: > >> Den tis 23 juni 2026 kl 15:34 skrev <[email protected]>: >> >>> Author: ivan >>> Date: Tue Jun 23 13:33:58 2026 >>> New Revision: 1935586 >>> >>> Log: >>> On 'xml-schema-validation-improvements' branch: >>> >>> Always use installed lxml and rnc2rng Python packages to validate XML >>> output >>> of `svn --xml` in tests. >>> >>> This change also makes these package mandatory for running tests unless >>> new >>> option `--disable-xml-schema-validation` is specified. >>> >> >> [...] >> >> I had a few minutes to spare this afternoon so I checked this commit and >> did three tests on Ubuntu 26.04: >> - With python3-lxml install: make check succceeded >> - Without python3-lxml installed: make check failed with error messages, >> this in the summary: >> [[[ >> There were some XML validation errors, >> checking/home/dsg/svn_branches/xml-schema-validation-improvements/tests.log >> XML: Module lxml.etree not found >> ]]] >> - Without python3-lxml: DISABLE_XML_SCHEMA_VALIDATION=true make check >> succeed. >> >> I think this looks good - fails by default but can be made to work. >> >> I also tried cmake/ctest but that failed spectacularly with (62 tests >> failed) or without lxml (61 tests failed!), so I presume something else is >> wrong. >> >> Only thing remaining is to document this. "make check" is only mentioned >> under "Building from a Tarball". Maybe it deserves a separate section ("D. >> Running the test suite"). Possibly even restructuring "II. Installation" to >> separate our three build systems (autoconf/make, vcproj and CMake) and >> having a separate section on testing under each? Do we do this on the >> branch or after merging back to trunk? >> >> > Thanks for testing this! > > There are a few things that still need to be done: > > 1. I learned that we can actually use the builtin Python XML package to > check the structural validity of XML responses. While this isn't the same > as fully validating the response against a schema definition, it still lets > us catch many cases where we produce invalid XML. > > In r1935590 I implemented this improvement and I'm now thinking about > making this mode the default, enabling full schema validation only if > explicitly requested. This way maintainers that run tests in their > pipelines can still get an adequate level of checking without being > required to install additional packages. And we ourselves will enable the > full validation in GitHub Actions. > > > > I like this approach, with one small nit – there's no need to explicitly > request schema validation, just check at runtime if the required modules > (lxml and rng2rnc) are available and if they are, perform full validation. > There is/was already code in svntest/main.py to check that. > > Since we now have the always-on XML structural check, I'd say we should pick between two options: A) Make full schema validation implicit when the modules are available. B) Have an explicit `--enable-xml-schema-validation` switch to enable it. I'm fine with either of these approaches. However, the explicit `--enable-xml-schema-validation` approach has a couple of advantages: - It makes test results, such as error messages, predictable and independent of the environment. - It gives users a way to check whether their environment is suitable for full XML schema validation. If a user passes `--enable-xml-schema-validation` and the modules aren't present, that's an error rather than a downgrade of the check. That said, I'm also okay with the implicit approach (A) that you described. [..] > 2. On Mac OS, there is an issue where FindPython3() in CMake locates a > different Python binary that doesn't match `python` from the command line > [1]. > > I still need to sort this part out. > > > > It probably finds /usr/bin/python3 by default, which is Python 3.9 on > recent versions of macOS. I think you'll have to set the Python3_ROOT_DIR > hint. Given the python binary you get on the command line, try this: > > python3 -c 'import pathlib, sys; > print(str(pathlib.Path(sys.executable).resolve().parent.parent))' > > > Haven't tested this yet, but thanks, it looks promising. -- Ivan Zhakov
