Skip to content

pass-an_optional_classification_value_passes_if_specified passes vacuously #456

Description

@BIMvoice

pass-an_optional_classification_value_passes_if_specified does not test what its name says. It passes whether or not classification value matching works at all.

What the fixture contains

#1=IFCPROJECT('1hqIFTRjfV6AWq_bMtnZwI',$,$,$,$,$,$,$,$);
#2=IFCCLASSIFICATION($,$,$,'ExpectedValue',$,$,$);
#3=IFCRELASSOCIATESCLASSIFICATION('05rScmOVzMoQXOfbYdtLYj',$,$,$,(#1),#2);
#4=IFCWALL('3Agm079vPIYBL4JExVrhD5',$,$,$,$,$,$,$,$);

The specification applies to IFCWALL, so the element under test is #4. But #3's RelatedObjects is (#1), the IfcProject, not (#4). The wall has no classification association at all.

Why it passes

The requirement uses Optional cardinality. With no classification present on the element, the optional branch short-circuits to pass before any value comparison happens. The 'ExpectedValue' set on #2 is never read, because nothing ever reaches the value check.

Confirmed by measurement against IfcOpenShell's IfcTester: ifcopenshell.util.classification.get_references(wall) returns an empty set, while the same call on the project returns {#2}.

So the case would still pass if classification value matching were entirely broken. A pass alone here cannot be distinguished from "never checked".

The suggested fix

Point #3's RelatedObjects at (#4) so the wall actually carries the classification, which makes the case exercise the value match it is named for.

Related

The sibling pass-an_optional_classification_value_passes_if_null has the same structure, but there it is harmless: the point of that case is that the element has no classification, so the association pointing elsewhere does not change what is being tested. It is only misleading in the _if_specified case.

Noticed while measuring #417 (where @atomczak has already given the correct explanation for the _if_null case). Filing separately so it does not get lost in that thread, since it concerns a different fixture.

Happy to open a PR with the one line change if that is useful.

This issue was filed with the assistance of an AI coding tool.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions