buildingSMART
pass-occurrences_override_the_type_classification_per_system_1_3
I think for this test case there is one IFCWALL too many. I think #4 should be removed because it makes the testcase fail.
Probably the same thing for pass-occurrences_override_the_type_classification_per_system_3_3.
Created an issue as PRs will eventually get overwritten.
There is a /* Testcase */ marker for the line relevant to the example.
But in general, why are the examples for Classification so long and full of "junk"? Could we shorten this example like for the attribute test cases etc. (without /* Testcase */ mark unless absolutely necessary). It would make it much more readable? It's often quite confusing. The other test cases (Attribute etc.) are very smart.
That marker is not in the actual test case IFC file (the ones in https://github.com/buildingSMART/IDS/tree/master/Documentation/testcases).
I hope the marker is only there for us human beings, not for the computer to recognize.
A few types could be omitted from this example I guess, but you definitely want to keep the occurrence and the type there.
Yes, I hope so too the marker is only there for us human beings. :-)
I don't only mean this test case. For example in test case [FAIL] A classification facet with no data matches any classification 1/2
#1=IFCPROJECT('1hqIFTRjfV6AWq_bMtnZwI',$,$,$,$,$,$,$,$);
#2=IFCCLASSIFICATION($,$,$,'Foobar',$,$,$);
#3=IFCRELASSOCIATESCLASSIFICATION('05rScmOVzMoQXOfbYdtLYj',$,$,$,(#1),#2);
#4=IFCWALL('3Agm079vPIYBL4JExVrhD5',$,$,$,$,$,$,$,$);
#5=IFCCLASSIFICATIONREFERENCE($,'1',$,#2,$,$);
would be enough, if I haven't missed anything.
I agree, the trick is that from fail/pass in the file name you should be able to detect the expected outcome. A comment marker to highlight a specific focus entity is indeed not very portable.
@Moult any ideas on a quick fix? Are there any cases where multiple products in the file is a necessity?
Just to clarify, in my experience for >90% of the testcases the fail/pass indicator in the filename does seem to cover the whole IDS, so any extra entities are not interfering with that. Got to say I really appreciate all the effort that must have gone into this @Moult (and @aothms I presume?).
I think I fixed the file a couple of commits back without being aware of this issue. Could somebody review the current state of the file in the PR #240 and close this if appropriate?
Confirmed this is partially fixed with PR #240
There's now only one IfcWall in the 'pass' test files pass-occurrences_override_the_type_classification_per_system_1_3 and pass-occurrences_override_the_type_classification_per_system_3_3
However the extra wall still exists in fail-occurrences_override_the_type_classification_per_system_2_3 and I think it's possible this may invalidate the test and create false negatives. E.g. The test is expected to fail because the requirement is 'All elements of entity Wall should have a classification value 22' - and in this case the wall instance over-rides the value to '11' - but actually the test will always fail due to the extraneous wall that also does not meet the requirement.
Did you spotfix the IFC files or fix them at source @CBenghi ?
More generally I think these test IFCs for the Classification testcases need a slim down. There's a lot of redundant data in them from earlier tests which I think obscure the intent. e.g. just compare a property test with a classification test:
