-
Notifications
You must be signed in to change notification settings - Fork 156
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
Tests with texture and nullFeatureid missing #297
Comments
I agree that such a case could be added. A bit more generally, this is related to #226 . One could consider roughly the approaches:
Right now, the latter is used. And I think that there is some benefit in having the actual file with the actual issue. (A bit snarky: I'm pretty sure that there is a considerable number of "unit tests" in the world that do not really test functionality - in some cases they are testing whether something was mocked in a certain way. It's an art, and I'm by no means claiming that I'm doing the right thing, but for the validator, using that "file-based" level of tests seemed to make sense) One drawback: There may be many files, and there could be quite some redundancy. For example, the case that you described will be validated by the In the more "classical" unit-test approach, this function would be covered explicitly and only once (with all combinations of inputs) with something like
I'll keep this open as a reminder, even though increasing the test coverage is an ongoing task. |
In the meshFeatures testcases (https:/CesiumGS/3d-tiles-validator/tree/main/specs/data/gltfExtensions/meshFeatures), I'm missing tests with textures and nullFeatureId, might be worthwhile to add
The text was updated successfully, but these errors were encountered: