feat(cli): Improve validation test coverage and robustness for 'jing' (#79)

Improve validation test coverage and robustness for 'jing'

---------

Signed-off-by: sushant-suse <[email protected]>

Author: sushant-suse

changelog.d/79.feature.rst

@@ -0,0 +1 @@
+Support additional RNG validation with lxml. This provides an alternative when jing is not available or preferred.

src/docbuild/cli/cmd_validate/__init__.py

@@ -19,15 +19,26 @@
     nargs=-1,
     type=click.Path(exists=True, dir_okay=False, path_type=Path),
 )
+@click.option(
+    '--validation-method',
+    type=click.Choice(['jing', 'lxml'], case_sensitive=False),
+    default='jing',
+    help="Choose validation method: 'jing' or 'lxml'",
+)
 @click.pass_context
-def validate(ctx: click.Context, xmlfiles: tuple | Iterator[Path]) -> None:
+def validate(ctx: click.Context, xmlfiles: tuple | Iterator[Path], validation_method: str) -> None:
     """Subcommand to validate XML configuration files.
 
     :param ctx: The Click context object.
+    :param xmlfiles: XML files to validate, if empty all XMLs in config dir are used.
+    :param validation_method: Validation method to use, 'jing' or 'lxml'.
     """
     context: DocBuildContext = ctx.obj
+
+    # Set the chosen validation method in the context for downstream use
+    context.validation_method = validation_method.lower()
+
     if context.envconfig is None:
-        # log.critical('No envconfig found in context.')
         raise ValueError('No envconfig found in context.')
 
     if (paths := ctx.obj.envconfig.get('paths')) is None:
@@ -44,7 +55,7 @@ def validate(ctx: click.Context, xmlfiles: tuple | Iterator[Path]) -> None:
     else:
         xml_files_to_process = xmlfiles
 
-    log.info('Validating XML configuration files')
+    log.info(f'Validating XML configuration files using {validation_method} method')
 
     result = asyncio.run(process_mod.process(context, xmlfiles=xml_files_to_process))
 

src/docbuild/cli/cmd_validate/process.py

@@ -168,6 +168,48 @@ async def validate_rng(
         )
 
 
+def validate_rng_lxml(
+    xmlfile: Path,
+    rng_schema_path: Path = PRODUCT_CONFIG_SCHEMA,
+) -> tuple[bool, str]:
+    """
+    Validate an XML file against a RELAX NG (.rng) schema using lxml.
+
+    This function uses lxml.etree.RelaxNG, which supports only the XML syntax
+    of RELAX NG schemas (i.e., .rng files, not .rnc files).
+
+    :param xmlfile: Path to the XML file to validate.
+    :param rng_schema_path: Path to the RELAX NG (.rng) schema file.
+    :return: Tuple of (is_valid: bool, error_message: str)
+    """
+    try:
+        # Parse the RELAX NG schema from .rng file
+        relaxng_doc = etree.parse(rng_schema_path)
+        relaxng = etree.RelaxNG(relaxng_doc)
+
+        # Parse the XML file to validate
+        xml_doc = etree.parse(xmlfile)
+
+        # Perform validation
+        is_valid = relaxng.validate(xml_doc)
+        if is_valid:
+            return True, ''
+        else:
+            # Return validation error log as string
+            return False, str(relaxng.error_log)
+
+    # Catch specific exceptions for better error handling
+    except etree.XMLSyntaxError as e:
+        # This handles syntax errors in either the XML or the RNG file
+        return False, f'XML or RNG syntax error: {e}'
+    except etree.RelaxNGParseError as e:
+        # This handles errors in parsing the RNG schema itself
+        return False, f'RELAX NG schema parsing error: {e}'
+    except Exception as e:
+        # This catch-all is a fallback for any other unexpected issues
+        return False, f'An unexpected error occurred during validation: {e}'
+
+
 async def run_python_checks(
     tree: etree._ElementTree,
 ) -> list[tuple[str, CheckResult]]:
@@ -209,7 +251,26 @@ async def process_file(
     # IDEA: Should we replace jing and validate with etree.RelaxNG?
 
     # 1. RNG Validation
-    rng_success, rng_output = await validate_rng(path_obj)
+    validation_method = context.validation_method
+
+    if validation_method == 'lxml':
+        # Use lxml-based validator (requires .rng schema)
+        rng_success, rng_output = await asyncio.to_thread(
+            validate_rng_lxml,
+            path_obj,
+            XMLDATADIR / 'product-config-schema.rng',  
+        )
+    elif validation_method == 'jing':
+        # Use existing jing-based validator (.rnc or .rng)
+        rng_success, rng_output = await validate_rng(path_obj)
+    else:
+        console_err.print(
+            f'{shortname:<{max_len}}: RNG validation => [red]failed[/red]'
+        )
+        console_err.print(f'  [bold red]Error:[/] Unknown validation method: {validation_method}')
+        return 11  # Custom error code for unknown validation method
+
+    # Handle validation result
     if not rng_success:
         console_err.print(
             f'{shortname:<{max_len}}: RNG validation => [red]failed[/red]'

src/docbuild/cli/context.py

@@ -40,3 +40,6 @@ class DocBuildContext:
 
     debug: bool = False
     """If set, enable debug mode"""
+
+    validation_method: str = "jing"
+    """Method used to validate XML files: 'jing' (default) or 'lxml'"""

tests/cli/cmd_validate/test_process.py

@@ -6,21 +6,20 @@
 from lxml import etree
 import pytest
 
-# Import the module to allow patching with patch.object
 from docbuild.cli.cmd_validate import process as process_module
 from docbuild.cli.cmd_validate.process import (
     process,
     process_file,
     registry,
     run_python_checks,
+    validate_rng_lxml,
 )
 from docbuild.cli.context import DocBuildContext
 from docbuild.config.xml.checks import CheckResult
 
 
 @pytest.fixture
 def mock_context() -> DocBuildContext:
-    """Fixture for a mocked DocBuildContext."""
     context = MagicMock(spec=DocBuildContext)
     context.verbose = 3
     context.envconfig = {
@@ -29,38 +28,29 @@ def mock_context() -> DocBuildContext:
             'base_server_cache_dir': '/fake/cache',
         }
     }
+    context.validation_method = 'jing'
     return context
 
 
-@patch.object(
-    process_module, 'validate_rng', new_callable=AsyncMock, return_value=(True, '')
-)
-@patch.object(process_module.asyncio, 'to_thread')
+@patch.object(process_module, 'validate_rng', new_callable=AsyncMock, return_value=(True, ''))
+@patch.object(process_module.etree, 'parse', side_effect=ValueError('Generic test error'))
 async def test_process_file_with_generic_parsing_error(
-    mock_to_thread: MagicMock,
-    mock_validate_rng: AsyncMock,
-    mock_context: DocBuildContext,
-    tmp_path: Path,
-    capsys: pytest.CaptureFixture,
+    mock_etree_parse, mock_validate_rng, mock_context, tmp_path, capsys
 ):
-    """Test process_file with a generic exception during parsing.
-
-    This covers the `except Exception` block.
-    """
-    mock_to_thread.side_effect = ValueError('Generic test error')
     xml_file = tmp_path / 'file.xml'
     xml_file.touch()
 
     exit_code = await process_file(xml_file, mock_context, max_len=20)
 
     assert exit_code == 200
+    mock_validate_rng.assert_awaited_once()
+    mock_etree_parse.assert_called_once()
     captured = capsys.readouterr()
     assert 'Error:' in captured.err
     assert 'Generic test error' in captured.err
 
 
-async def test_process_no_envconfig(mock_context: DocBuildContext):
-    """Test that process raises ValueError if envconfig is missing."""
+async def test_process_no_envconfig(mock_context):
     mock_context.envconfig = None
     with pytest.raises(ValueError, match='No envconfig found in context.'):
         await process(mock_context, xmlfiles=(Path('dummy.xml'),))
@@ -69,21 +59,57 @@ async def test_process_no_envconfig(mock_context: DocBuildContext):
 @patch.object(process_module, 'process_file', new_callable=AsyncMock, return_value=0)
 @patch.object(process_module, 'create_stitchfile', new_callable=AsyncMock)
 async def test_process_with_stitchfile_failure(
-    mock_create_stitchfile: AsyncMock,
-    mock_process_file: AsyncMock,
-    mock_context: DocBuildContext,
-    capsys: pytest.CaptureFixture,
+    mock_create_stitchfile, mock_process_file, mock_context, capsys
 ):
-    """Test process function when create_stitchfile raises a ValueError.
-
-    This covers the `except ValueError` block during stitch-file validation.
-    """
     mock_create_stitchfile.side_effect = ValueError('Duplicate product IDs found')
-    xml_files = (Path('/fake/file1.xml'),)  # Need at least one successful file
+    xml_files = (Path('/fake/file1.xml'),)
 
     exit_code = await process(mock_context, xmlfiles=xml_files)
 
     assert exit_code == 1
     captured = capsys.readouterr()
     assert 'Stitch-file validation failed:' in captured.err
     assert 'Duplicate product IDs found' in captured.err
+
+
+@patch.object(process_module, 'validate_rng_lxml', new_callable=MagicMock, return_value=(True, ''))
+@patch.object(process_module, 'etree')
+@patch.object(process_module, 'run_python_checks', new_callable=AsyncMock)
+async def test_process_file_with_lxml_validation_success(
+    mock_run_checks, mock_etree, mock_validate_lxml, mock_context, tmp_path
+):
+    xml_file = tmp_path / 'file.xml'
+    xml_file.touch()
+    mock_context.validation_method = 'lxml'
+    mock_etree.parse.return_value = etree.ElementTree(etree.Element('root'))
+    mock_run_checks.return_value = [(None, CheckResult(success=True))]
+
+    exit_code = await process_file(xml_file, mock_context, max_len=20)
+
+    assert exit_code == 0
+    mock_validate_lxml.assert_called_once()
+    mock_run_checks.assert_awaited_once()
+
+
+@patch.object(process_module, 'validate_rng_lxml', new_callable=MagicMock, return_value=(False, 'error message'))
+async def test_process_file_with_lxml_validation_failure(
+    mock_validate_rng_lxml, mock_context, tmp_path
+):
+    xml_file = tmp_path / 'file.xml'
+    xml_file.touch()
+    mock_context.validation_method = 'lxml'
+
+    exit_code = await process_file(xml_file, mock_context, max_len=20)
+
+    assert exit_code == 10
+    mock_validate_rng_lxml.assert_called_once()
+
+
+async def test_process_file_with_unknown_validation_method(mock_context, tmp_path):
+    xml_file = tmp_path / 'file.xml'
+    xml_file.touch()
+    mock_context.validation_method = 'unknown_method'
+
+    exit_code = await process_file(xml_file, mock_context, max_len=20)
+
+    assert exit_code == 11

tests/cli/cmd_validate/validate/test_cmd_validate.py

@@ -88,6 +88,7 @@ def mock_context(self):
         context = Mock(spec=DocBuildContext)
         context.envconfig = {'paths': {'config_dir': '/test/config'}}
         context.verbose = 1
+        context.validation_method = "jing"
         return context
 
     @pytest.fixture