test: improve guards for experimental features

NAPI_EXPERIMENTAL would be expand to NAPI_VERSION=2147483647 if
NAPI_VERSION is not defined. It would be not compatible with various
Node.js versions if we use NAPI_VERSION > 2147483646 mixed with
definition of NAPI_EXPERIMENTAL. This fix introduced NODE_MAJOR_VERSION
to allow more precise control over test sets that which set should be
enabled on a Node.js release.

PR-URL: #545
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>

.travis.yml

@@ -56,13 +56,7 @@ install:
 script:
   # Travis CI sets NVM_NODEJS_ORG_MIRROR, but it makes node-gyp fail to download headers for nightly builds.
   - unset NVM_NODEJS_ORG_MIRROR
-  - NODEJS_MAJOR_VERSION=$(node -p "process.versions.node.match(/\d+/)[0]")
 
-  - |
-    if [ ${NODEJS_MAJOR_VERSION} -gt 11 ]; then
-      npm test
-    else
-      npm test $NPMOPT --NAPI_VERSION=$(node -p "process.versions.napi")
-    fi
+  - npm test
 after_success:
   - cpp-coveralls --gcov-options '\-lp' --build-root test/build --exclude test

napi-inl.h

@@ -383,9 +383,10 @@ inline bool Value::IsNumber() const {
   return Type() == napi_number;
 }
 
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with the definition of NAPI_EXPERIMENTAL.
+// Once it is no longer experimental guard with the NAPI_VERSION in which it is
+// released instead.
+#ifdef NAPI_EXPERIMENTAL
 inline bool Value::IsBigInt() const {
   return Type() == napi_bigint;
 }
@@ -620,9 +621,10 @@ inline double Number::DoubleValue() const {
   return result;
 }
 
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with the definition of NAPI_EXPERIMENTAL.
+// Once it is no longer experimental guard with the NAPI_VERSION in which it is
+// released instead.
+#ifdef NAPI_EXPERIMENTAL
 ////////////////////////////////////////////////////////////////////////////////
 // BigInt Class
 ////////////////////////////////////////////////////////////////////////////////

napi.h

@@ -112,9 +112,10 @@ namespace Napi {
   class Value;
   class Boolean;
   class Number;
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with the definition of NAPI_EXPERIMENTAL.
+// Once it is no longer experimental guard with the NAPI_VERSION in which it is
+// released instead.
+#ifdef NAPI_EXPERIMENTAL
   class BigInt;
 #endif  // NAPI_EXPERIMENTAL
 #if (NAPI_VERSION > 4)
@@ -140,9 +141,10 @@ namespace Napi {
   typedef TypedArrayOf<uint32_t> Uint32Array; ///< Typed-array of unsigned 32-bit integers
   typedef TypedArrayOf<float> Float32Array;   ///< Typed-array of 32-bit floating-point values
   typedef TypedArrayOf<double> Float64Array;  ///< Typed-array of 64-bit floating-point values
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with the definition of NAPI_EXPERIMENTAL.
+// Once it is no longer experimental guard with the NAPI_VERSION in which it is
+// released instead.
+#ifdef NAPI_EXPERIMENTAL
   typedef TypedArrayOf<int64_t> BigInt64Array;   ///< Typed array of signed 64-bit integers
   typedef TypedArrayOf<uint64_t> BigUint64Array; ///< Typed array of unsigned 64-bit integers
 #endif  // NAPI_EXPERIMENTAL
@@ -245,9 +247,10 @@ namespace Napi {
     bool IsNull() const;        ///< Tests if a value is a null JavaScript value.
     bool IsBoolean() const;     ///< Tests if a value is a JavaScript boolean.
     bool IsNumber() const;      ///< Tests if a value is a JavaScript number.
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with the definition of NAPI_EXPERIMENTAL.
+// Once it is no longer experimental guard with the NAPI_VERSION in which it is
+// released instead.
+#ifdef NAPI_EXPERIMENTAL
     bool IsBigInt() const;      ///< Tests if a value is a JavaScript bigint.
 #endif  // NAPI_EXPERIMENTAL
 #if (NAPI_VERSION > 4)
@@ -322,9 +325,10 @@ namespace Napi {
     double DoubleValue() const;   ///< Converts a Number value to a 64-bit floating-point value.
   };
 
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with the definition of NAPI_EXPERIMENTAL.
+// Once it is no longer experimental guard with the NAPI_VERSION in which it is
+// released instead.
+#ifdef NAPI_EXPERIMENTAL
   /// A JavaScript bigint value.
   class BigInt : public Value {
   public:
@@ -852,9 +856,10 @@ namespace Napi {
         : std::is_same<T, uint32_t>::value ? napi_uint32_array
         : std::is_same<T, float>::value ? napi_float32_array
         : std::is_same<T, double>::value ? napi_float64_array
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with the definition of NAPI_EXPERIMENTAL.
+// Once it is no longer experimental guard with the NAPI_VERSION in which it is
+// released instead.
+#ifdef NAPI_EXPERIMENTAL
         : std::is_same<T, int64_t>::value ? napi_bigint64_array
         : std::is_same<T, uint64_t>::value ? napi_biguint64_array
 #endif  // NAPI_EXPERIMENTAL

test/bigint.cc

@@ -1,11 +1,13 @@
+// Currently experimental guard with NODE_MAJOR_VERISION in which it was
+// released. Once it is no longer experimental guard with the NAPI_VERSION
+// in which it is released instead.
+#if (NODE_MAJOR_VERSION >= 10)
+
 #define NAPI_EXPERIMENTAL
 #include "napi.h"
 
 using namespace Napi;
 
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
 namespace {
 
 Value IsLossless(const CallbackInfo& info) {

test/binding.cc

@@ -1,4 +1,3 @@
-#define NAPI_EXPERIMENTAL
 #include "napi.h"
 
 using namespace Napi;
@@ -14,9 +13,10 @@ Object InitBasicTypesArray(Env env);
 Object InitBasicTypesBoolean(Env env);
 Object InitBasicTypesNumber(Env env);
 Object InitBasicTypesValue(Env env);
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with NODE_MAJOR_VERISION in which it was
+// released. Once it is no longer experimental guard with the NAPI_VERSION
+// in which it is released instead.
+#if (NODE_MAJOR_VERSION >= 10)
 Object InitBigInt(Env env);
 #endif
 Object InitBuffer(Env env);
@@ -63,9 +63,10 @@ Object Init(Env env, Object exports) {
   exports.Set("basic_types_boolean", InitBasicTypesBoolean(env));
   exports.Set("basic_types_number", InitBasicTypesNumber(env));
   exports.Set("basic_types_value", InitBasicTypesValue(env));
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with NODE_MAJOR_VERISION in which it was
+// released. Once it is no longer experimental guard with the NAPI_VERSION
+// in which it is released instead.
+#if (NODE_MAJOR_VERSION >= 10)
   exports.Set("bigint", InitBigInt(env));
 #endif
 #if (NAPI_VERSION > 4)

test/binding.gyp

@@ -1,6 +1,7 @@
 {
   'variables': {
-    'NAPI_VERSION%': "",
+    'NAPI_VERSION%': "<!(node -p \"process.versions.napi\")",
+    'NODE_MAJOR_VERSION%': "<!(node -p \"process.versions.node.match(/\\d+/)[0]\")",
     'disable_deprecated': "<!(node -p \"process.env['npm_config_disable_deprecated']\")"
   },
   'target_defaults': {
@@ -59,6 +60,7 @@
           }
         }]
       ],
+      'defines': ['NODE_MAJOR_VERSION=<@(NODE_MAJOR_VERSION)'],
       'include_dirs': ["<!@(node -p \"require('../').include\")"],
       'dependencies': ["<!(node -p \"require('../').gyp\")"],
       'cflags': [ '-Werror', '-Wall', '-Wextra', '-Wpedantic', '-Wunused-parameter' ],

test/date.cc

@@ -1,4 +1,3 @@
-#define NAPI_EXPERIMENTAL
 #include "napi.h"
 
 using namespace Napi;

test/index.js

@@ -50,39 +50,38 @@ let testModules = [
   'version_management'
 ];
 
-if ((process.env.npm_config_NAPI_VERSION !== undefined) &&
-    (process.env.npm_config_NAPI_VERSION < 50000)) {
-  // currently experimental only test if NAPI_VERSION
-  // is set to experimental. We can't use C max int
-  // as that is not supported as a number on earlier
-  // Node.js versions. Once bigint is in a release
-  // this should be guarded on the napi version
-  // in which bigint was added.
+const napiVersion = Number(process.versions.napi)
+const nodeMajorVersion = Number(process.versions.node.match(/\d+/)[0])
+
+if (nodeMajorVersion < 10) {
+  // Currently experimental guard with NODE_MAJOR_VERISION in which it was
+  // released. Once it is no longer experimental guard with the NAPI_VERSION
+  // in which it is released instead.
   testModules.splice(testModules.indexOf('bigint'), 1);
   testModules.splice(testModules.indexOf('typedarray-bigint'), 1);
 }
 
-if ((process.env.npm_config_NAPI_VERSION !== undefined) &&
-    (process.env.npm_config_NAPI_VERSION < 3)) {
+if (napiVersion < 3) {
   testModules.splice(testModules.indexOf('callbackscope'), 1);
   testModules.splice(testModules.indexOf('version_management'), 1);
 }
 
-if ((process.env.npm_config_NAPI_VERSION !== undefined) &&
-    (process.env.npm_config_NAPI_VERSION < 4)) {
+if (napiVersion < 4) {
   testModules.splice(testModules.indexOf('asyncprogressworker'), 1);
   testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function_ptr'), 1);
   testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function_sum'), 1);
   testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function_unref'), 1);
   testModules.splice(testModules.indexOf('threadsafe_function/threadsafe_function'), 1);
 }
 
-if ((process.env.npm_config_NAPI_VERSION !== undefined) &&
-    (process.env.npm_config_NAPI_VERSION < 5)) {
+if (napiVersion < 5) {
   testModules.splice(testModules.indexOf('date'), 1);
 }
 
 if (typeof global.gc === 'function') {
+  console.log(`Testing with N-API Version '${napiVersion}'.`);
+  console.log(`Testing with Node.js Major Version '${nodeMajorVersion}'.\n`);
+
   console.log('Starting test suite\n');
 
   // Requiring each module runs tests in the module.

test/typedarray.cc

@@ -1,4 +1,9 @@
+// Currently experimental guard with NODE_MAJOR_VERISION in which it was
+// released. Once it is no longer experimental guard with the NAPI_VERSION
+// in which it is released instead.
+#if (NODE_MAJOR_VERSION >= 10)
 #define NAPI_EXPERIMENTAL
+#endif
 #include "napi.h"
 
 using namespace Napi;
@@ -65,9 +70,10 @@ Value CreateTypedArray(const CallbackInfo& info) {
       NAPI_TYPEDARRAY_NEW(Float64Array, info.Env(), length, napi_float64_array) :
       NAPI_TYPEDARRAY_NEW_BUFFER(Float64Array, info.Env(), length, buffer, bufferOffset,
                                  napi_float64_array);
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with NODE_MAJOR_VERISION in which it was
+// released. Once it is no longer experimental guard with the NAPI_VERSION
+// in which it is released instead.
+#if (NODE_MAJOR_VERSION >= 10)
   } else if (arrayType == "bigint64") {
     return buffer.IsUndefined() ?
       NAPI_TYPEDARRAY_NEW(BigInt64Array, info.Env(), length, napi_bigint64_array) :
@@ -101,9 +107,10 @@ Value GetTypedArrayType(const CallbackInfo& info) {
     case napi_uint32_array: return String::New(info.Env(), "uint32");
     case napi_float32_array: return String::New(info.Env(), "float32");
     case napi_float64_array: return String::New(info.Env(), "float64");
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with NODE_MAJOR_VERISION in which it was
+// released. Once it is no longer experimental guard with the NAPI_VERSION
+// in which it is released instead.
+#if (NODE_MAJOR_VERSION >= 10)
     case napi_bigint64_array: return String::New(info.Env(), "bigint64");
     case napi_biguint64_array: return String::New(info.Env(), "biguint64");
 #endif
@@ -143,9 +150,10 @@ Value GetTypedArrayElement(const CallbackInfo& info) {
       return Number::New(info.Env(), array.As<Float32Array>()[index]);
     case napi_float64_array:
       return Number::New(info.Env(), array.As<Float64Array>()[index]);
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with NODE_MAJOR_VERISION in which it was
+// released. Once it is no longer experimental guard with the NAPI_VERSION
+// in which it is released instead.
+#if (NODE_MAJOR_VERSION >= 10)
     case napi_bigint64_array:
       return BigInt::New(info.Env(), array.As<BigInt64Array>()[index]);
     case napi_biguint64_array:
@@ -189,9 +197,10 @@ void SetTypedArrayElement(const CallbackInfo& info) {
     case napi_float64_array:
       array.As<Float64Array>()[index] = value.DoubleValue();
       break;
-// currently experimental guard with version of NAPI_VERSION that it is
-// released in once it is no longer experimental
-#if (NAPI_VERSION > 2147483646)
+// Currently experimental guard with NODE_MAJOR_VERISION in which it was
+// released. Once it is no longer experimental guard with the NAPI_VERSION
+// in which it is released instead.
+#if (NODE_MAJOR_VERSION >= 10)
     case napi_bigint64_array: {
       bool lossless;
       array.As<BigInt64Array>()[index] = value.As<BigInt>().Int64Value(&lossless);