Fix #5950. IfcTester now considers optional requirement facets as per latest documentation.

The IDS documentation used to state that "optional" on requirement
facets was "for information only" and didn't affect the pass / fail
state. This was changed a while ago (which I missed) to a new behaviour:
both no values and passing values will pass.

src/ifctester/ifctester/facet.py

@@ -279,16 +279,13 @@ class Attribute(Facet):
         return results
 
     def __call__(self, inst: ifcopenshell.entity_instance, logger: Optional[Logger] = None) -> AttributeResult:
-        if self.cardinality == "optional":
-            return AttributeResult(True)
-
         if isinstance(self.name, str):
             names = [self.name]
             attribute_type = inst.wrapped_data.get_attribute_category(self.name)
             if attribute_type == 1:  # Forward attribute
                 values = [getattr(inst, self.name, None)]
             else:
-                values = [None]
+                values = []
         else:
             info = inst.get_info()
             names = []
@@ -304,6 +301,8 @@ class Attribute(Facet):
         reason = None
 
         if not is_pass:
+            if self.cardinality == "optional":
+                return AttributeResult(True)
             reason = {"type": "NOVALUE"}
 
         if is_pass:
@@ -394,9 +393,6 @@ class Classification(Facet):
         return ifc_file.by_type("IfcObjectDefinition")
 
     def __call__(self, inst: ifcopenshell.entity_instance, logger: Optional[Logger] = None) -> ClassificationResult:
-        if self.cardinality == "optional":
-            return ClassificationResult(True)  # Is this really the correct behaviour?
-
         leaf_references = ifcopenshell.util.classification.get_references(inst)
 
         references = leaf_references.copy()
@@ -407,6 +403,8 @@ class Classification(Facet):
         reason = None
 
         if not is_pass:
+            if self.cardinality == "optional":
+                return ClassificationResult(True)
             reason = {"type": "NOVALUE"}
 
         if is_pass and self.value:
@@ -657,9 +655,6 @@ class Property(Facet):
         )
 
     def __call__(self, inst: ifcopenshell.entity_instance, logger: Optional[Logger] = None) -> PropertyResult:
-        if self.cardinality == "optional":
-            return PropertyResult(True)
-
         if isinstance(self.propertySet, str):
             pset = get_pset(inst, self.propertySet)
             psets = {self.propertySet: pset} if pset else {}
@@ -671,6 +666,8 @@ class Property(Facet):
         reason = None
 
         if not is_pass:
+            if self.cardinality == "optional":
+                return PropertyResult(True)
             reason = {"type": "NOPSET"}
 
         if is_pass:
@@ -691,6 +688,8 @@ class Property(Facet):
                     props[pset_name] = {k: v for k, v in pset_props.items() if k == self.baseName}
 
                 if not bool(props[pset_name]):
+                    if self.cardinality == "optional":
+                        return PropertyResult(True)
                     is_pass = False
                     reason = {"type": "NOVALUE"}
                     break
@@ -918,15 +917,14 @@ class Material(Facet):
         return ifc_file.by_type("IfcObjectDefinition")
 
     def __call__(self, inst: ifcopenshell.entity_instance, logger: Optional[Logger] = None) -> MaterialResult:
-        if self.cardinality == "optional":
-            return MaterialResult(True)
-
         material = ifcopenshell.util.element.get_material(inst, should_skip_usage=True)
 
         is_pass = material is not None
         reason = None
 
         if not is_pass:
+            if self.cardinality == "optional":
+                return MaterialResult(True)
             reason = {"type": "NOVALUE"}
 
         if is_pass and self.value:

src/ifctester/test/test_facet.py

@@ -259,9 +259,19 @@ class TestAttribute:
         facet = Attribute(name="Name", cardinality="prohibited")
         run("A prohibited facet returns the opposite of a required facet", facet=facet, inst=element, expected=False)
         facet = Attribute(name="Name", cardinality="optional")
-        run("An optional facet always passes regardless of outcome 1/2", facet=facet, inst=element, expected=True)
+        run(
+            "An optional facet only checks requirements if there is a value to check 1/2",
+            facet=facet,
+            inst=element,
+            expected=True,
+        )
         facet = Attribute(name="Rabbit", cardinality="optional")
-        run("An optional facet always passes regardless of outcome 2/2", facet=facet, inst=element, expected=True)
+        run(
+            "An optional facet only checks requirements if there is a value to check 2/2",
+            facet=facet,
+            inst=element,
+            expected=True,
+        )
 
         ifc = ifcopenshell.file()
         facet = Attribute(name="Name")
@@ -750,9 +760,19 @@ class TestClassification:
         facet = Classification(system="Foobar", cardinality="prohibited")
         run("A prohibited facet returns the opposite of a required facet", facet=facet, inst=element1, expected=False)
         facet = Classification(system="Foobar", cardinality="optional")
-        run("An optional facet always passes regardless of outcome 1/2", facet=facet, inst=element0, expected=True)
+        run(
+            "An optional facet only checks requirements if there is a value to check 1/2",
+            facet=facet,
+            inst=element0,
+            expected=True,
+        )
         facet = Classification(system="Foobar", cardinality="optional")
-        run("An optional facet always passes regardless of outcome 2/2", facet=facet, inst=element1, expected=True)
+        run(
+            "An optional facet only checks requirements if there is a value to check 2/2",
+            facet=facet,
+            inst=element1,
+            expected=True,
+        )
 
         facet = Classification(system="Foobar", value="1")
         run(
@@ -887,9 +907,19 @@ class TestProperty:
         facet = Property(propertySet="Foo_Bar", baseName="Foo", dataType="IFCLABEL", cardinality="prohibited")
         run("A prohibited facet returns the opposite of a required facet", facet=facet, inst=element, expected=False)
         facet = Property(propertySet="Foo_Bar", baseName="Foo", dataType="IFCLABEL", cardinality="optional")
-        run("An optional facet always passes regardless of outcome 1/2", facet=facet, inst=element, expected=True)
+        run(
+            "An optional facet only checks requirements if there is a value to check 1/2",
+            facet=facet,
+            inst=element,
+            expected=True,
+        )
         facet = Property(propertySet="Foo_Bar", baseName="Bar", dataType="IFCLABEL", cardinality="optional")
-        run("An optional facet always passes regardless of outcome 2/2", facet=facet, inst=element, expected=True)
+        run(
+            "An optional facet only checks requirements if there is a value to check 2/2",
+            facet=facet,
+            inst=element,
+            expected=True,
+        )
 
         ifcopenshell.api.run("pset.edit_pset", ifc, pset=pset, properties={"Foo": ""})
         facet = Property(propertySet="Foo_Bar", baseName="Foo", dataType="IFCLOGICAL")
@@ -1278,9 +1308,19 @@ class TestMaterial:
         facet = Material(cardinality="prohibited")
         run("A prohibited facet returns the opposite of a required facet", facet=facet, inst=element, expected=False)
         facet = Material(cardinality="optional")
-        run("An optional facet always passes regardless of outcome 1/2", facet=facet, inst=element, expected=True)
+        run(
+            "An optional facet only checks requirements if there is a value to check 1/2",
+            facet=facet,
+            inst=element,
+            expected=True,
+        )
         facet = Material(value="Foo", cardinality="optional")
-        run("An optional facet always passes regardless of outcome 1/2", facet=facet, inst=element, expected=True)
+        run(
+            "An optional facet only checks requirements if there is a value to check 1/2",
+            facet=facet,
+            inst=element,
+            expected=False,
+        )
 
         ifc = ifcopenshell.file()
         facet = Material(value="Foo")

src/ifctester/test/test_ids.py

@@ -51,7 +51,7 @@ def run(
 
 class TestIds:
     def test_failing_on_opening_invalid_ids_data(self):
-        with pytest.raises(xmlschema.validators.exceptions.XMLSchemaValidationError):
+        with pytest.raises(ids.IdsXmlValidationError):
             ids.open("""<?xml version="1.0" encoding="UTF-8"?><clearly_not_an_ids/>""")
 
     def test_create_an_ids_with_minimal_information(self):