Skip to content

Add partOf depth-2 recursion test cases (#155) - #457

Open
BIMvoice wants to merge 2 commits into
buildingSMART:developmentfrom
BIMvoice:propose/ids-155-partof-recursion-depth2-testcases
Open

Add partOf depth-2 recursion test cases (#155)#457
BIMvoice wants to merge 2 commits into
buildingSMART:developmentfrom
BIMvoice:propose/ids-155-partof-recursion-depth2-testcases

Conversation

@BIMvoice

@BIMvoice BIMvoice commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Adds conformance coverage for a gap that let a real implementation bug survive.

The gap

Of the 33 official partOf conformance cases, only 3 exercise same-relation recursion beyond depth 1, and none covers IFCRELASSIGNSTOGROUP or IFCRELCONTAINEDINSPATIALSTRUCTURE beyond a direct relation. One of those 3, pass-the_containment_can_be_indirect_1_2, is in fact a pure IFCRELAGGREGATES chain despite its name, so containment recursion was not exercised anywhere in the suite.

Why it matters

That is exactly where IfcOpenShell's IfcTester had a bug. Its group and containment branches inspected only the element's single direct relation, so an element two hops away silently passed a PROHIBITED requirement, reporting compliance while the relationship existed. Fixed in IfcOpenShell#9272.

Documentation/UserManual/partof-facet.md on development states the rule these cases pin:

When the relation parameter is not specified, then all 6 are to be considered (recursively) to identify containing entities, otherwise only the specified relation type should be considered (also recursively).

Note that file differs between development and ver/1.1.x; the sentence above is from development.

What is added

Two pass and fail pairs, each pair sharing a two-hop chain with distinct intermediate and target classes at every hop so an entity type collision cannot mask the result:

  • IFCRELASSIGNSTOGROUP: pass-a_group_relationship_may_be_indirect and fail-a_prohibited_group_relationship_detects_an_indirect_group
  • IFCRELCONTAINEDINSPATIALSTRUCTURE: pass-a_containment_relationship_may_be_indirect and fail-a_prohibited_containment_relationship_detects_an_indirect_container

Verification

Run against IfcTester both before and after the fix above:

pre-fix   the 4 new cases mismatch their own filename; the 33 existing cases are unaffected
post-fix  0 of 37 mismatch

So the cases demonstrably catch the defect, and demonstrably do not disturb the existing suite.

All 4 .ids files validate against Schema/ids.xsd, checked with xmlschema. A control file using relation="NOTAVALIDRELATION" is correctly rejected, confirming the validator discriminates rather than passing anything.

Correction: I originally wrote that CreateTestCases could not be run. That was wrong. The build needs the .NET 8 runtime specifically, because Nuke uses BinaryFormatter, which .NET 9 removed. With that installed it runs in about a second. These .ids files are still hand-authored; I am happy to regenerate them through the official build on request.

Scope

This does not address the separate "require a direct relationship" question also raised on #155, which is still open. #155 is assigned to @CBenghi, so this is offered as a starting point to take, change or discard.

This PR was created with the assistance of an AI coding tool.

Of the 33 official partOf conformance cases, only 3 exercise
same-relation recursion beyond depth 1, and none covers
IFCRELASSIGNSTOGROUP or IFCRELCONTAINEDINSPATIALSTRUCTURE beyond a
direct relation. One of the three, pass-the_containment_can_be_indirect_1_2,
is in fact a pure IFCRELAGGREGATES chain, so containment recursion was
not exercised anywhere in the suite.

This gap is exactly where the IfcOpenShell IfcTester implementation
had a bug: its group and containment branches only inspected the
element's single direct relation, so an element two hops away
silently passed a PROHIBITED requirement. Fixed in IfcOpenShell PR
#9272.

Adds two pass/fail pairs, one per uncovered relation, each pairing a
required and a prohibited specification over the same two-hop chain
with distinct intermediate and target classes at every hop, per
Documentation/UserManual/partof-facet.md (development branch): "When
the relation parameter is not specified, then all 6 are to be
considered (recursively) ... otherwise only the specified relation
type should be considered (also recursively)."

This file was generated with the assistance of an AI coding tool.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

1 participant