Tests with texture and nullFeatureid missing about 3d-tiles-validator HOT 1 OPEN

I agree that such a case could be added.

A bit more generally, this is related to #226 . One could consider roughly the approaches:

• write "classical" unit tests, that set up some data + mocks, and check the result of calling a certain function
• use the more "holistic" ("integration-level") approach of trying to create one invalid file for each possible validation issue

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 FeatureIdValidator. And this will use the same code path, regardless of whether the feature IDs come from an attribute or from a texture. So one could create a test for feature ID textures and a nullFeatureId, but it will "only" be another test of this particular function.

In the more "classical" unit-test approach, this function would be covered explicitly and only once (with all combinations of inputs) with something like

const featureIdSet = ...;
const nullFeatureId = ...;
FeatureIdValidator.validateFeatureIdSet(... featureIdSet, nullFeatureId, mockContext);
expect(mockContext.issues.length).toBe(1);

I'll keep this open as a reminder, even though increasing the test coverage is an ongoing task.

from 3d-tiles-validator.