Add error handling for invalid/unknown parameter type

[medley56]

Handle Errors for Unsupported/Invalid Parameter Types

• Add error handling for unknown and invalid parameter type element during XTCE parsing

Checklist

• Changes are fully implemented without dangling issues or TODO items
• Deprecated/superseded code is removed or marked with deprecation warning
• Current dependencies have been properly specified and old dependencies removed
• New code/functionality has accompanying tests and any old tests have been updated to match any new assumptions
• The changelog.md has been updated

[greglucas]

Why is this needed? It looks like you've already added it to the __init__()

This seems like it could be confusing/a gotcha if we are required to call private methods in downstream things to clear/reinitialize anything.

[medley56]

Oh good catch. I was playing around to see if we could get away with not populating the whole cache ahead of time (do it on the first packet parse) but it's necessary to sort out container inheritance so I added it back into the init method.

Will fix.

[greglucas]

Is there an easy way to manipulate this through code rather than adding these xml files? I had a hard time finding the invalid portion here because there is so much base verbosity you need to define a valid XTCE in the first place, whereas if you could do something like (wrapping it all in a fixture or something else to help automate some of the cleanup)

# Load in a base/template xtce
test_xtce = load_xml()
# Replace a single entry you want in the tree with your invalid portion
test_xtce["parameter_type"].name = "INVALID_NAME"
# Write this out to a temporary buffer
with bytesIO() as f:
    test_xtce.write(f)
    # Reload with this package's loading routine
    f.seek(0)
    xdef = xtcedef.XtcePacketDefinition(f, ns=TEST_NAMESPACE)

[medley56]

Hmm this is an interesting idea. I'll explore this.

[medley56]

I haven't polished it but this is what I came up with as a first pass. To be honest, this is super ugly and I'm not sure it's worth it. If I can make this a little more streamlined I'll be happier. I also think I now need to make a more generic test_xtce.xml file that is more suitable to these changes.

def test_invalid_parameter_type_error(test_data_dir):
    """Test proper reporting of an invalid parameter type element"""
    with io.BytesIO() as temp:
        with open(test_data_dir / 'test_xtce.xml', mode='r') as x:
            etree = ElementTree.parse(x)
            parameter_type_set = etree.find(".//xtce:ParameterTypeSet", TEST_NAMESPACE)
            invalid_type = ElementTree.Element(f"{{{XTCE_URI}}}InvalidParameterType")
            invalid_type.set("name", "TEST_INVALID_Type")
            parameter_type_set.append(invalid_type)

            parameter_set = etree.find(".//xtce:ParameterSet", TEST_NAMESPACE)
            invalid_parameter_ref = ElementTree.Element(f"{{{XTCE_URI}}}Parameter")
            invalid_parameter_ref.set("name", "TEST_INVALID")
            invalid_parameter_ref.set("parameterTypeRef", "TEST_INVALID_Type")
            parameter_set.append(invalid_parameter_ref)

            entry_list = etree.find(".//xtce:SequenceContainer[@name='JPSS_ATT_EPHEM']/xtce:EntryList", TEST_NAMESPACE)
            parameter_instance = ElementTree.Element(f"{{{XTCE_URI}}}ParameterRefEntry")
            parameter_instance.set("parameterRef", "TEST_INVALID")
            entry_list.append(parameter_instance)
        etree.write(temp)

        temp.seek(0)
        with pytest.raises(xtcedef.InvalidParameterTypeError):
            xtcedef.XtcePacketDefinition(temp, ns=TEST_NAMESPACE)

[greglucas]

I agree, that isn't a lot more descriptive than the added XTCE files you had, and hopefully there aren't a lot of extra files that we need to make for the various test scenarios.

One additional thing in case it sparks some idea for you is whether you can simplify the test to only go through the single method you're interested in testing rather than parsing the full document.

def test_parse_sequence_container():
    # Open a base test file to give you a good instance
    with open(test_data_dir / 'test_xtce.xml', mode='r') as x:
        packet_def = xtcedef.XtcePacketDefinition(temp, ns=TEST_NAMESPACE)

    # Create a simple ETree subsection for your individual test
    small_etree_sequence_container = etree.ETree(
        """<xtce:ParameterRefType><other inline elements ...>"""
    )
    with pytest.raises():
        packet_def.parse_sequence_container(small_etree_sequence_container)

[medley56]

Yeah, that's a good thought. The problem there is that the changes I need to make to the document are in all different places, not in a contiguous chunk. Still, that notion would simplify the fixture a bit.

I haven't given up on this yet because I would like to reduce the number of test data files whenever possible.

[medley56]

I want to get the testing right for this so I'm moving this PR to the 4.3.0 milestone so I can release 4.2.0