test(math.vector): add tests for GetAngleBetweenVectorsOnPlane function

[Dok11]

I generated 1000 variants of usage the GetAngleBetweenVectorsOnPlane function base on these values:

const possibleValues = [
        // negative values
        Number.MIN_SAFE_INTEGER,
        Number.MIN_SAFE_INTEGER / 2,
        -Math.PI,
        -0.0000000001,

        0,

        // positive values
        0.0000000001,
        Math.PI,
        Number.MAX_SAFE_INTEGER / 2,
        Number.MAX_SAFE_INTEGER,
    ];

So I got time comutation statistics that added to jsdoc.

What do you think about unit tests like this? Or probably we should set just several examples without edge values (like max_safe_integer)?

[azure-pipelines]

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

[azure-pipelines]

Snapshot stored with reference name:
refs/pull/12974/merge

Test environment:
https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12974/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12974/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/12974/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/12974/merge
https://gui.babylonjs.com/?snapshot=refs/pull/12974/merge
https://nme.babylonjs.com/?snapshot=refs/pull/12974/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/12974/merge#BCU1XR#0

[RaananW]

Thanks for that!

I wonder - how were these numbers generated? And why do we need 1000 tests and not a few fixed (and tested) ones, including edge cases?

[Dok11]

I wonder - how were these numbers generated? And why do we need 1000 tests and not a few fixed (and tested) ones, including edge cases?

Yes, I want to discourse! =)

As edge cases I take Number.MIN_INTEGER_VALUE and max. Also I take zero and value close to zero 0.00000001 (negative and positive).
Also I choose some normal value between zero and max value — Math.PI (negative and positive).
In fact, I also added half value of Number.MIN and MAX_INTEGER_VALUE as normal value, but currenly I think it was redundant because Math.PI do same work.
So at least we have possible values:

const possibleValues = [
  Number.MIN_SAFE_INTEGER,
  -Math.PI,
  -0.0000000001,
  0,
  0.0000000001,
  Math.PI,
  Number.MAX_SAFE_INTEGER,
]

The function take 3 input Vector3 (3 input by 3 value), so to calculate all possible test cases we should Math.pow(possibleValues.length, 3 * 3); it equal to 40 353 607. Of course it is a lot. So I generate by random take value and got 1000 test cases.

Is all of them required? It is hard question. But I got some interesting insight.
After create these tests I start to modify the function and what I found - if we remove v0.normalize() and v1.normalize() then just a several tests of 1000 will failed, other of them works fine.
It's mean for me, these test keep guarantee to the function works well in most common tasks.

[RaananW]

I wonder - how were these numbers generated? And why do we need 1000 tests and not a few fixed (and tested) ones, including edge cases?

Yes, I want to discourse! =)

As edge cases I take Number.MIN_INTEGER_VALUE and max. Also I take zero and value close to zero 0.00000001 (negative and positive). Also I choose some normal value between zero and max value — Math.PI (negative and positive). In fact, I also added half value of Number.MIN and MAX_INTEGER_VALUE as normal value, but currenly I think it was redundant because Math.PI do same work. So at least we have possible values:

const possibleValues = [
  Number.MIN_SAFE_INTEGER,
  -Math.PI,
  -0.0000000001,
  0,
  0.0000000001,
  Math.PI,
  Number.MAX_SAFE_INTEGER,
]

The function take 3 input Vector3 (3 input by 3 value), so to calculate all possible test cases we should Math.pow(possibleValues.length, 3 * 3); it equal to 40 353 607. Of course it is a lot. So I generate by random take value and got 1000 test cases.

Is all of them required? It is hard question. But I got some interesting insight. After create these tests I start to modify the function and what I found - if we remove v0.normalize() and v1.normalize() then just a several tests of 1000 will failed, other of them works fine. It's mean for me, these test keep guarantee to the function works well in most common tasks.

I will go over it in depth in an hour or two :-)

How were they generated? using the same function?

[Dok11]

How were they generated? using the same function?

No, just simple for loop. I dont save it but it was looking like:

for (let i = 0; i < 1000; i++) {
  testCases.push({
    v0: { x: getRandomPossibleValue(), /* y: ... */ },
    // same for
    // v1: 
    // normal
  });
}

after run it I fill the result and save it as fixed test cases as array in this PR.

[RaananW]

How were they generated? using the same function?

No, just simple for loop. I dont save it but it was looking like:

for (let i = 0; i < 1000; i++) {
  testCases.push({
    v0: { x: getRandomPossibleValue(), /* y: ... */ },
    // same for
    // v1: 
    // normal
  });
}

after run it I fill the result and save it as fixed test cases as array in this PR.

I mean the results :-) You are testing the function against the results it has generated, right? So this test is only testing if the function has changed? Or have you generated the results using some other method?

[Dok11]

I mean the results :-) You are testing the function against the results it has generated, right? So this test is only testing if the function has changed? Or have you generated the results using some other method?

Right, I got current results by current state of the function with trust that it work correct.

So this test is only testing if the function has changed?

Yep.
If function work incorrect then tests wrong too.

What do you think, which is the best way to write unit tests in such case?

[RaananW]

I mean the results :-) You are testing the function against the results it has generated, right? So this test is only testing if the function has changed? Or have you generated the results using some other method?

Right, I got current results by current state of the function with trust that it work correct.

So this test is only testing if the function has changed?

Yep. If function work incorrect then tests wrong too.

What do you think, which is the best way to write unit tests in such case?

Nope, I just wanted to be sure what is being tested :-)

[Dok11]

I just thought that this way to testing is good for long living high-order functions and I want to discourse it with you, what is the best way? Leave public functions without any tests is not good definitely, but how to test them right?

[Dok11]

And what do you think? Will some similar PRs be helpful if I make some of them? For me, it is an interesting way to get to know framework.

[RaananW]

Of course. I would rather not have 1000 test cases (10-20 would be great :-)), but we are always happy for these kinds of PRs!

[Dok11]

I would rather not have 1000 test cases (10-20 would be great :-))

I understand you, but this situation looks like a potential problem when we talking about limited test cases:

After create these tests I start to modify the function and what I found - if we remove v0.normalize() and v1.normalize() then just a several tests of 1000 will failed, other of them works fine.