129 add a schema validator checker - #180
Conversation
2de35de to
d50a2f0
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #180 +/- ##
==========================================
- Coverage 94.19% 93.98% -0.21%
==========================================
Files 40 42 +2
Lines 2841 3375 +534
==========================================
+ Hits 2676 3172 +496
- Misses 165 203 +38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
greglucas
left a comment
There was a problem hiding this comment.
High-level this looks OK. Some thoughts here:
If you are calling "validate_xtce()", do I really want to have to check the return after it? I'm more used to it raising if it is not valid, and then continuing on if it is. So by calling validate I kind of want you to raise an InvalidXTCEError. That could then contain all of the helpful information and maybe even print out within the message etc. That is a pretty large structural change from this approach, so figured I would put that at the top-level comment.
If you do keep it this current way with a "result" return, would you want to add an ability to make it evaluate as a bool so someone could say if if not validate_xtce() and go into the block that way?
Is there a way to automatically fix any of these suggestions and allow someone to do something on the command line like: --validate --fix which would overwrite or write a new version of the XTCE with the suggested changes/updates? That should be a future ticket, just a thought I had while reviewing this.
|
You make a great point about raising an exception and that's what I would rather do versus a Boolean behavior. I think I could refactor that pretty easily by building in the result check as part of validate_xtce. If result is invalid, attach the result to an exception and raise it. I'll play around with it. That part is easy compared to all the lxml calls for schema validation. The big thing is I don't want users to have to run the validator over and over uncovering one error at a time. |
e02c807 to
9062ed4
Compare
|
@greglucas Here's what's changed since you last reviewed. We're ready for another round. Infrastructure & Testing Improvements
XTCE Validation Enhancements
CLI & Error Handling
Documentation
|
greglucas
left a comment
There was a problem hiding this comment.
It is getting quite big for me to review now, but overall I think this is OK since it is only validation / one-time-use kinds of things, so I'll put down a few notes after trying it out.
- I'm not sure how much the validate() function will actually be used in practice, but I would have expected
raise_on_error=Trueto be the default. i.e. I can just see someone doingspp.validate_xtce(my_file) ... carry on because it is valid - If you are including the SpaceSystem.xsd file in the test_data, I wonder if it'd be worthwhile just including it with the actual release too at that point. It is rarely changed and ~300KB, so not the end of the world. There is a lot of caching/downloading logic involved. Doesn't make a huge difference to me either way though.
- I am getting errors saying the xsi header needs to be specified to validate an XTCE document. Is there a way you could validate a given version provided by the user or something like that instead of requiring that to be in the file itself? I realize maybe that isn't valid spec, but seems odd to fail with an error in that case to me too because our files are currently working just fine.
- I like the cli coloring and formating :)
fda7e3a to
ff022ec
Compare
- Ruff formatted and ruff lint fixes - Remove namespace on test_xtce_no_namespace.xml and make namespace handling on definitions.py clearer - Fix XML structure for XTCE test files - Fix CTIM XTCE for validity - Update changelog - Tighten up warnings for no namespace for XtcePacketDefinition - Add tests for XSD schema validity - Add tests for different inputs to validation function - Add test for different inputs to definition from_xtce - Add structural validation and tests - Add validate_xtce to top level module - Add simple CLI to validation tool - Add codecov.yaml - Mock out XSD retrieval from omg.org to prevent DOSing the schema server when testing - Add caching mechanism for XSD retrievals during validation - Clean up CLI handling of common error
ff022ec to
249e16a
Compare
Summary
This PR implements comprehensive XTCE document validation capabilities for Space
Packet Parser, adding both XSD schema validation and structural validation to help
ensure XTCE documents are correct and will work properly for parsing packets.
Changes Made
Core Validation Implementation
space_packet_parser/xtce/validation.py(441 lines) - Completevalidation system with:
ValidationLevelenum (schema, structure, all)ValidationErrorandValidationResultdataclasses for structured errorreporting
validate_xtce()function as main entry pointAPI Integration
space_packet_parser/__init__.py- Exportedvalidate_xtcefunctionfor public API
space_packet_parser/xtce/__init__.py- Renamed constants:DEFAULT_XTCE_NS_PREFIX→STANDARD_XTCE_NS_PREFIXDEFAULT_XTCE_NSMAP→STANDARD_XTCE_NSMAPCLI Enhancement
space_packet_parser/cli.py- Addedspp validatecommand with:--leveloption (schema/structure/all)--timeoutoption for schema downloadsImproved Namespace Handling
space_packet_parser/xtce/definitions.py- Major improvements tonamespace awareness:
Configuration Updates
pyproject.toml- Added Ruff ignore rule for f-strings in docstringsdocs/source/users.md- Added comprehensive validation documentationwith examples
Test Infrastructure
tests/unit/test_xtce/test_validation.py(348 lines) - Comprehensivetest suite:
tests/unit/test_xtce/test_definitions.py- Addedvalidation-related tests
tests/integration/test_cli.py- Added CLI validation command teststests/conftest.py- Updated constant referencesTest Data Improvements
tests/test_data/ctim/ctim_xtce_v1.xml- Fixed namespace and structure issuestests/test_data/test_xtce_no_namespace.xml- Removed namespace for testingKey Features
automatic downloading
and unused elements
spp validatecommand for easy validation from commandline
locations
detection
Test Coverage