Famous / engine

MIT License
1.75k stars 250 forks source link

Non-deterministic tests #364

Open alexanderGugel opened 9 years ago

alexanderGugel commented 9 years ago

Currently there are a couple of non-deterministic tests that should either be made deterministic, fixed or adjusted to log the generated random values.

Example

    t.test('GeometryHelper.normalizeVertices', function(t) {
        var vertices = generateRandomArray(10, [-10, 10]);
        var normalized = GeometryHelper.normalizeVertices(vertices);
        var withinRange = testVector(normalized, function(vector) {
            return (vector.x <= 1) && (vector.x >= -1)
                && (vector.y <= 1) && (vector.y >= -1)
                && (vector.z <= 1) && (vector.z >= -1);
        });

        t.ok(withinRange, "All vectors should be within 2x2 bounding box");
        t.end();
    });

in GeometryHelper.spec.js

The issue is var vertices = generateRandomArray(10, [-10, 10]). Sometimes it returns an array that makes the test fail, sometimes it doesn't (in which case we don't know about the values that made it fail). I've seen this before, but wasn't sure it was actually related to this test, but it seems to be the case. E.g. I'm specifically talking about the following CI build that passes on 0.10, but fails on iojs: https://travis-ci.org/Famous/engine/builds/68386307

@michaelobriena How do you want to proceed with this?

solomon-gumball commented 9 years ago

Hmm... thanks for pointing this out @alexanderGugel, I should probably fix this, though I can't possibly see how it could fail the test. This might be an issue with Vec3.normalize I'll look into it.

alexanderGugel commented 9 years ago

@redwoodfavorite Hmmm... I also tried to reproduce the failing test locally, but couldn't get it to fail. This seems very strange to me. In either case, testing using randomized values is fine IMO, but we should log them.

michaelobriena commented 9 years ago

@redwoodfavorite are you actively working on this?