Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review of some Test-Cases regarding 0.9.6 release #194

Open
MarcelStepien opened this issue Sep 14, 2023 · 3 comments
Open

Review of some Test-Cases regarding 0.9.6 release #194

MarcelStepien opened this issue Sep 14, 2023 · 3 comments
Assignees
Milestone

Comments

@MarcelStepien
Copy link

In regard to the 0.9.6 release I would like to make some comments regarding findings in the test-cases, specifically for tests concerning the Attribute- and Classification-Facet.

1) Attribute-Facet Test-Case:

  • fail - a prohibited facet returns the opposite of a required facet

This test-case lost its meaning, since the attribute-facet no longer contains min- and maxOccurs to represent the prohibited constraint.

2) Attribute-Facet Test-Case:

  • pass - an optional facet always passes regardless of outcome 2 2

This test-case lost its meaning, since the attribute-facet no longer contains min- and maxOccurs to represent the optional constraint.

There are certainly more cases for the attribute-facet that has to be reviewed based on the now missing min- and maxOccurs. The here listed cases are certain to now be broken.

3) Classification-Facet Test-Case:

  • pass - Occurrences override the type classification per system 1/3
  • pass - Occurrences override the type classification per system 3/3

The IFC for both classification tests are containing exactly two IFCWALL entities. One is compliant with the IDS, the other is not. This results in a failed check for the specification, not meeting the pass requirement of the test-case. To fix this, I would simply rename the following entity to fix the test-cases:

#4=IFCWALL... -> #4=IFCSLAB...

If not renaming the failing wall entity, it has to have a valid classification to resolve successful.

4) Classification-Facet Test-Case:

  • pass - Values match subreferences if full classifications are used (e.g. EF_25_10 should match EF_25_10_25, EF_25_10_30, etc)

This test-case tries to utilize a simpleValue constraint as if it is a regular expression. A simpleValue should remain a check for equivalence. To create a check that allows creating a "starts with" constraint, such as for EF_25_10 resolving successfull for EF_25_10_25 and EF_25_10_30, a RegEx pattern can be formulated and should be used instead, like this:

<requirements>
    <classification minOccurs="1" maxOccurs="unbounded">
        <value>
            <xs:restriction base="xs:string">
                <xs:pattern value="^EF_25_10.*" />
            </xs:restriction>
        </value>
    </classification>
</requirements>

I addition to that, the simple check for 2 or 22 as classification value does not comply with the description of the test-case.

5) All Test-Case:

It has to be noted, that all test cases currently use min- and maxOccurs with the values [1, 1]. In the documentation it is explicitly stated, that only combinations resulting in prohibited, optional and required are allowed. So, the combination [1, 1] for min- and maxOccurs is invalid!

@andyward
Copy link

I believe 1), 2) & hopefully 5) are getting fixed up in #192 (comment)

  1. caught us as well #4=IFCWALL('3Agm079vPIYBL4JExVrhD5',$,$,$,$,$,$,$,$); is definitely extraneous in those tests. I suspect that it got copied from other test models in the generation. It's logged as pass-occurrences_override_the_type_classification_per_system_1_3 #108

  2. I believe this is a correct test. It's checking that '2' matches a classification in the ancestry. The IfcBeam is classified to '22' which has a parent classification of '2'. Some of these Classification tests are a bit "noisy" I found, so it takes some working out the intent

@CBenghi
Copy link
Contributor

CBenghi commented Feb 28, 2024

Hi @MarcelStepien, can I ask you to review this in light of the the changes committed to #240?

@CBenghi CBenghi added this to the 1.0 milestone Feb 28, 2024
@MarcelStepien
Copy link
Author

@CBenghi I will be able to provide an overview of the state of test-cases once we converted all to the newer version of IDS.

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

No branches or pull requests

3 participants