test: improve test-arm-math-illegal-instruction.js

[marsonya]

add Math.clz32(x) to the test

[RaisinTen]

What do you think about doing something like this instead:

Object.getOwnPropertyNames(Math).forEach(function (functionName) {
  if (!/[A-Z]/.test(functionName)) {
    // The function names don't have capital letters.
    Math[functionName](-0.5);
  }
});

That way, we don't have to manually keep a track of all the function names.

[marsonya]

What do you think about doing something like this instead:

Object.getOwnPropertyNames(Math).forEach(function (functionName) {
  if (!/[A-Z]/.test(functionName)) {
    // The function names don't have capital letters.
    Math[functionName](-0.5);
  }
});

That way, we don't have to manually keep a track of all the function names.

This seems like a great idea.
But, for some reason, the test considers only single parameter functions.
Functions like min, max, pow, etc are excluded.

[RaisinTen]

I don't think that will be a problem given the purpose of the test:
https://github.com/nodejs/node/blob/f355549f7faa96f976284e178a7fbe5ac109350a/test/parallel/test-arm-math-illegal-instruction.js#L4-L7
I think it's just checking whether calling any of these Math functions end up failing with an illegal instruction error, so it does not really matter what we feed into these functions provided the error does not happen.

[marsonya]

I don't think that will be a problem given the purpose of the test:
https://github.com/nodejs/node/blob/f355549f7faa96f976284e178a7fbe5ac109350a/test/parallel/test-arm-math-illegal-instruction.js#L4-L7

I think it's just checking whether calling any of these Math functions end up failing with an illegal instruction error, so it does not really matter what we feed into these functions provided the error does not happen.

That sounds fair. The functions that require more than one parameters are simply returning NaN.

[marsonya]

What do you think about doing something like this instead:

Object.getOwnPropertyNames(Math).forEach(function (functionName) {
  if (!/[A-Z]/.test(functionName)) {
    // The function names don't have capital letters.
    Math[functionName](-0.5);
  }
});

That way, we don't have to manually keep a track of all the function names.

This seems a great solution to me. Let's wait for more reviews.
If there is consensus around this, I will change the test.

[aduh95]

+1 on @RaisinTen idea.

FWIW we could use _function_.length to determine how many parameters it expects (Math.abs.length === 1 and Math.pow.length === 2), although getting NaN seems OK too.

[marsonya]

New changes made as per @RaisinTen's idea.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/36714/

[aduh95]

Landed in 67d2262