comments update

Author: fabiomolinar

crates/powerlink-rs-xdc/tests/parsing.rs

@@ -1,7 +1,11 @@
-// crates/powerlink-rs-xdc/tests/parsing.rs
+//! Integration tests for full XDC/XDD file parsing and round-trip serialization.
+//!
+//! These tests validate that real-world (or mock real-world) XML files are
+//! parsed correctly into the public `XdcFile` structure and can be serialized
+//! back to XML without data loss.
 
 use powerlink_rs_xdc::{
-    ParameterAccess, load_xdc_from_str, load_xdd_defaults_from_str, save_xdc_to_string,
+    load_xdc_from_str, load_xdd_defaults_from_str, save_xdc_to_string, ParameterAccess,
 };
 use std::fs;
 use std::path::PathBuf;
@@ -17,82 +21,81 @@ fn load_test_file(name: &str) -> String {
         .unwrap_or_else(|e| panic!("Failed to read test file {:?}: {}", path, e))
 }
 
-/// This test validates that the resolver correctly loads data from the
-/// <ApplicationProcess> block and applies it to the <ObjectList>
-/// by resolving the `uniqueIDRef`.
+/// Validates that the resolver correctly loads data from the `<ApplicationProcess>`
+/// block and applies it to the `<ObjectList>` via `uniqueIDRef`.
 #[test]
 fn test_resolve_extended_app_process() {
     let xml_content = load_test_file("MyDevice_extended.xdd");
+    // Load as XDD (use defaults)
     let xdc_file = load_xdd_defaults_from_str(&xml_content).expect("Failed to parse extended XDD");
 
-    // 1. Find the Object Dictionary
     let od = &xdc_file.object_dictionary;
 
-    // 2. Find Object 0x2100, which references "ID_Parameter1"
+    // 1. Find Object 0x2100, which references "ID_Parameter1"
     let obj_2100 = od
         .objects
         .iter()
         .find(|o| o.index == 0x2100)
         .expect("Failed to find object 0x2100");
 
-    // 3. Assert that its attributes were resolved from "ID_Parameter1"
+    // 2. Assert that its attributes were resolved from "ID_Parameter1"
     assert_eq!(obj_2100.name, "ExampleSimpleParameter_U8");
     assert_eq!(
         obj_2100.access_type,
         Some(ParameterAccess::ReadWrite),
         "Access type was not resolved from <parameter>"
     );
-    // Assert that the `defaultValue` ("15") was correctly parsed and resolved
-    // from the <parameter> (as USINT, "0005").
-    // UPDATED: We now expect the string "15" directly from the XML.
+    // Assert that the `defaultValue` ("15") was correctly parsed from the <parameter>.
     assert_eq!(
         obj_2100.data.as_deref(),
         Some("15"),
         "defaultValue was not resolved from <parameter>"
     );
 
-    // 4. Find Object 0x2101, which references "ID_Parameter2" (a struct)
+    // 3. Find Object 0x2101, which references "ID_Parameter2" (a struct)
     let obj_2101 = od
         .objects
         .iter()
         .find(|o| o.index == 0x2101)
         .expect("Failed to find object 0x2101");
 
-    // Assert its attributes were resolved
     assert_eq!(obj_2101.name, "ExampleStructure_DOM");
     assert_eq!(
         obj_2101.access_type,
         Some(ParameterAccess::ReadWrite),
         "Access type was not resolved from <parameter>"
     );
-    // The parameter itself has no value, so the data should be None
+    // The parameter itself has no value, so data should be None
     assert_eq!(obj_2101.data, None);
 }
 
-/// This test validates the full "round-trip" capability.
-/// 1. Load XDD (from `defaultValue`)
-/// 2. Save to string (as XDC, serializing data as `actualValue`)
-/// 3. Load the new string (from `actualValue`)
-/// 4. Assert the two resulting structs are identical.
+/// Validates the full "round-trip" capability.
+///
+/// 1. Load an XDD file (parsing `defaultValue`).
+/// 2. Serialize it back to a string (generating valid XDC XML).
+/// 3. Load that new string back in.
+/// 4. Assert the internal structures match.
 #[test]
 fn test_round_trip_static_xdd() {
-    // 1. Load the original XDD, parsing `defaultValue`
+    // 1. Load original XDD
     let xdd_content = load_test_file("MyDevice_static.xdd");
     let file1 =
         load_xdd_defaults_from_str(&xdd_content).expect("Failed to parse original static XDD");
 
-    // 2. Save it back to a new XDC-style string
-    // This converts `file1.data` (from `defaultValue`) into `actualValue` attributes.
+    // 2. Serialize to string
     let xdc_string_new = save_xdc_to_string(&file1).expect("Failed to serialize XDC to string");
 
-    // 3. Load the *new* string, parsing `actualValue`
+    // 3. Load the new string
+    // Note: `save_xdc_to_string` generally writes values to `actualValue` fields
+    // if loaded from XDC, or preserves source structure.
+    // For this test, we use `load_xdc_from_str` to read the result.
     let file2 = load_xdc_from_str(&xdc_string_new).expect("Failed to parse newly serialized XDC");
 
-    // 4. Detailed Comparisons for Debugging
+    // 4. Detailed Comparisons
     assert_eq!(file1.header, file2.header, "Header mismatch");
     assert_eq!(file1.identity, file2.identity, "Identity mismatch");
 
-    // Check NetworkManagement specifically
+    // Check NetworkManagement
     if let (Some(nm1), Some(nm2)) = (&file1.network_management, &file2.network_management) {
         assert_eq!(
             nm1.general_features, nm2.general_features,
@@ -141,7 +144,7 @@ fn test_round_trip_static_xdd() {
     assert_eq!(file1, file2, "XdcFile structs mismatch after round-trip");
 }
 
-/// This test simply ensures the minimal "dynamic" XDD parses correctly.
+/// Validates that the minimal "dynamic" XDD parses correctly.
 #[test]
 fn test_load_dynamic_xdd() {
     let xml_content = load_test_file("MyDevice.xdd");
@@ -162,7 +165,5 @@ fn test_load_dynamic_xdd() {
         .expect("Object 0x1F82 not found");
 
     assert_eq!(flags_obj.name, "NMT_FeatureFlags_U32");
-    // `defaultValue="0x00000045"`
-    // UPDATED: We now expect the string "0x00000045" directly from the XML.
     assert_eq!(flags_obj.data.as_deref(), Some("0x00000045"));
-}
+}

crates/powerlink-rs-xdc/tests/robustness.rs

@@ -1,8 +1,12 @@
-// crates/powerlink-rs-xdc/tests/robustness.rs
+//! Integration tests focused on error handling and edge cases.
+//!
+//! These tests ensure the parser correctly identifies and reports errors for
+//! malformed XML, invalid attributes, missing mandatory elements, and data type
+//! mismatches, without panicking.
 
 use powerlink_rs_xdc::{load_xdc_from_str, to_core_od, XdcError};
 
-/// A minimal valid XDC template to be corrupted by tests.
+/// A minimal valid XDC template used as a base for creating corrupted test cases.
 const MINIMAL_VALID_XML: &str = r#"<?xml version="1.0" encoding="UTF-8"?>
 <ISO15745ProfileContainer xmlns="http://www.ethernet-powerlink.org" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.ethernet-powerlink.org Powerlink_Main.xsd">
   <ISO15745Profile>
@@ -57,7 +61,7 @@ const MINIMAL_VALID_XML: &str = r#"<?xml version="1.0" encoding="UTF-8"?>
   </ISO15745Profile>
 </ISO15745ProfileContainer>"#;
 
-/// Test that the parser catches malformed XML syntax (e.g. unclosed tags).
+/// Verifies that the parser catches malformed XML syntax (e.g., unclosed tags).
 #[test]
 fn test_malformed_xml_syntax() {
     let xml = r#"<ISO15745ProfileContainer><ProfileHeader> ... missing closing tags"#;
@@ -69,7 +73,7 @@ fn test_malformed_xml_syntax() {
     );
 }
 
-/// Test that the resolver catches invalid hex strings in the `index` attribute.
+/// Verifies that the resolver catches invalid hex strings in the `index` attribute.
 #[test]
 fn test_invalid_index_hex() {
     // Inject an invalid index "ZZZZ"
@@ -88,13 +92,15 @@ fn test_invalid_index_hex() {
     );
 }
 
-/// Test that the resolver handles missing ApplicationLayers gracefully or returns error.
-/// In our implementation, `resolver/mod.rs` explicitly checks for this.
+/// Verifies that the resolver requires the `ApplicationLayers` block in the Communication Profile.
 #[test]
 fn test_missing_application_layers() {
     // Remove the ApplicationLayers block
     let start = MINIMAL_VALID_XML.find("<ApplicationLayers>").unwrap();
-    let end = MINIMAL_VALID_XML.find("</ApplicationLayers>").unwrap() + "</ApplicationLayers>".len();
+    let end = MINIMAL_VALID_XML
+        .find("</ApplicationLayers>")
+        .unwrap()
+        + "</ApplicationLayers>".len();
     let mut xml = MINIMAL_VALID_XML.to_string();
     xml.replace_range(start..end, "");
 
@@ -111,17 +117,19 @@ fn test_missing_application_layers() {
     );
 }
 
-/// Test that the `converter` (to core OD) handles invalid data values gracefully.
-/// e.g. "INVALID" for a Unsigned16.
+/// Verifies that the `converter` (to core OD) handles invalid data values gracefully.
+///
+/// The parser loads the string as-is, but the converter must fail if the string
+/// cannot be parsed into the target native type.
 #[test]
 fn test_data_type_conversion_failure() {
     // Inject invalid data "NOT_HEX" for U16 type
     let xml = MINIMAL_VALID_XML.replace(r#"actualValue="0x1234""#, r#"actualValue="NOT_HEX""#);
-    
-    // 1. Load should succeed (the parser stores it as a String, which is valid XML).
+
+    // 1. Load should succeed (the parser stores it as a String).
     let xdc_file = load_xdc_from_str(&xml).expect("Parser should handle raw strings");
 
-    // 2. Conversion to Core OD should FAIL because "NOT_HEX" cannot become U16.
+    // 2. Conversion to Core OD should FAIL.
     let core_result = to_core_od(&xdc_file);
 
     assert!(
@@ -136,31 +144,30 @@ fn test_data_type_conversion_failure() {
     );
 }
 
-/// Test that the `converter` handles boolean parsing correctly (true/false/1/0).
+/// Verifies that the `converter` handles boolean parsing correctly across variants.
 #[test]
 fn test_boolean_conversion_variants() {
-    // Prepare an XML with a Boolean object
     let base_xml = MINIMAL_VALID_XML.replace(
         r#"<defType dataType="0006"><Unsigned16/></defType>"#,
-        r#"<defType dataType="0001"><Boolean/></defType>"#
+        r#"<defType dataType="0001"><Boolean/></defType>"#,
     ).replace(
         r#"dataType="0006" actualValue="0x1234""#,
-        r#"dataType="0001" actualValue="REPLACE_ME""#
+        r#"dataType="0001" actualValue="REPLACE_ME""#,
     );
 
-    // Case 1: "true"
+    // Case 1: "true" -> 1
     let xml_true = base_xml.replace("REPLACE_ME", "true");
     let file_true = load_xdc_from_str(&xml_true).unwrap();
     let od_true = to_core_od(&file_true).unwrap();
     let val_true = od_true.read_object(0x1000).unwrap();
-    // Core OD stores Boolean as u8 (1 or 0)
+    
     if let powerlink_rs::od::Object::Variable(powerlink_rs::od::ObjectValue::Boolean(v)) = val_true {
         assert_eq!(*v, 1);
     } else {
         panic!("Expected Boolean(1)");
     }
 
-    // Case 2: "0"
+    // Case 2: "0" -> 0
     let xml_zero = base_xml.replace("REPLACE_ME", "0");
     let file_zero = load_xdc_from_str(&xml_zero).unwrap();
     let od_zero = to_core_od(&file_zero).unwrap();
@@ -171,63 +178,62 @@ fn test_boolean_conversion_variants() {
         panic!("Expected Boolean(0)");
     }
 
-    // Case 3: Invalid "yes"
+    // Case 3: Invalid "yes" -> Error
     let xml_invalid = base_xml.replace("REPLACE_ME", "yes");
     let file_invalid = load_xdc_from_str(&xml_invalid).unwrap();
     let res_invalid = to_core_od(&file_invalid);
     assert!(matches!(res_invalid, Err(XdcError::InvalidAttributeFormat { .. })));
 }
 
-/// Test resilience against broken `uniqueIDRef` links.
-/// The resolver should currently ignore broken links (fail-open) or handle them without panic.
+/// Verifies resilience against broken `uniqueIDRef` links.
+///
+/// The resolver should treat broken links as "no value found" rather than panicking.
 #[test]
 fn test_broken_parameter_reference() {
-    // Inject a uniqueIDRef that doesn't exist
     let xml = MINIMAL_VALID_XML.replace(
         r#"<Object index="1000" name="Device Type" objectType="7" dataType="0006" actualValue="0x1234" />"#,
         r#"<Object index="1000" name="Device Type" objectType="7" uniqueIDRef="NON_EXISTENT_ID" />"#
     );
 
     let result = load_xdc_from_str(&xml);
     assert!(result.is_ok(), "Parser should not panic on broken ref");
-    
+
     let file = result.unwrap();
     let obj = &file.object_dictionary.objects[0];
-    
+
     // Since the ref was broken and no direct value provided, data should be None
     assert_eq!(obj.data, None);
 }
 
-/// Test that XML entities are correctly decoded (e.g., `B&amp;R` -> `B&R`).
+/// Verifies that XML entities are correctly decoded.
 #[test]
 fn test_xml_entity_decoding() {
     let xml = MINIMAL_VALID_XML.replace(
         r#"<vendorName>TestVendor</vendorName>"#,
-        r#"<vendorName>B&amp;R Automation</vendorName>"#
+        r#"<vendorName>B&amp;R Automation</vendorName>"#,
     );
 
     let xdc_file = load_xdc_from_str(&xml).expect("Failed to parse XML with entities");
     assert_eq!(xdc_file.identity.vendor_name, "B&R Automation");
 }
 
-/// Test that numeric values overflow their target types triggers an error.
-/// E.g. "256" for Unsigned8.
+/// Verifies that numeric values overflowing their target types trigger an error.
 #[test]
 fn test_numeric_overflow() {
-    // Set type to Unsigned8 (0005) but value to 256 (too large for u8)
+    // Set type to Unsigned8 (0005) but value to 256
     let base_xml = MINIMAL_VALID_XML.replace(
         r#"<defType dataType="0006"><Unsigned16/></defType>"#,
-        r#"<defType dataType="0005"><Unsigned8/></defType>"#
+        r#"<defType dataType="0005"><Unsigned8/></defType>"#,
     ).replace(
         r#"dataType="0006" actualValue="0x1234""#,
-        r#"dataType="0005" actualValue="256""#
+        r#"dataType="0005" actualValue="256""#,
     );
 
     let xdc_file = load_xdc_from_str(&base_xml).expect("Initial parse should succeed");
-    
+
     // Conversion should fail
     let core_result = to_core_od(&xdc_file);
-    
+
     assert!(
         matches!(
             core_result,
@@ -240,13 +246,12 @@ fn test_numeric_overflow() {
     );
 }
 
-/// Test missing mandatory fields in ProfileHeader.
+/// Verifies behavior when mandatory fields in ProfileHeader are missing.
 #[test]
 fn test_missing_header_fields() {
-    // Remove ProfileIdentification
     let xml = MINIMAL_VALID_XML.replace(
-        r#"<ProfileIdentification>Test</ProfileIdentification>"#, 
-        ""
+        r#"<ProfileIdentification>Test</ProfileIdentification>"#,
+        "",
     );
 
     let result = load_xdc_from_str(&xml);
@@ -258,27 +263,19 @@ fn test_missing_header_fields() {
     );
 }
 
-/// Test behavior when an unknown data type ID is used.
+/// Verifies behavior when an unknown data type ID is used.
 #[test]
 fn test_unknown_data_type() {
     // Use a made-up data type ID "FFFF"
-    let xml = MINIMAL_VALID_XML.replace(
-        r#"dataType="0006""#, 
-        r#"dataType="FFFF""#
-    );
+    let xml = MINIMAL_VALID_XML.replace(r#"dataType="0006""#, r#"dataType="FFFF""#);
 
     let xdc_file = load_xdc_from_str(&xml).expect("Parse should succeed");
-    
-    // Converter attempts to map this.
-    // Current implementation in converter.rs: map_data_to_value returns Ok(None) for unknown type.
-    // map_object sees None for a VAR (type 7) and returns XdcError::ValidationError.
+
+    // Converter attempts to map this, fails to find logic for "FFFF", returns error.
     let core_result = to_core_od(&xdc_file);
 
     assert!(
-        matches!(
-            core_result,
-            Err(XdcError::ValidationError(_))
-        ),
+        matches!(core_result, Err(XdcError::ValidationError(_))),
         "Expected ValidationError for unknown data type, got {:?}",
         core_result
     );