test: restore the validate parity test and close coverage gaps (#8)

Signed-off-by: Bryan English <bryan@bryanenglish.com>

Author: bengl

lib/validate.js

@@ -3,14 +3,8 @@
 // Faithful port of the small subset of Node.js core validators used by
 // this package (lib/internal/validators.js, lib/internal/errors.js),
 // matching core's error shapes (name, code, message) exactly.
-// TODO(bengl): add back the parity test that proved it. Spawn a child
-// with --expose-internals that runs one shared battery of calls through
-// require('internal/validators') and the ERR_INVALID_ARG_TYPE,
-// ERR_INVALID_ARG_VALUE, and ERR_OUT_OF_RANGE constructors from
-// internal/errors, print the outcomes (thrown or not, and the thrown
-// error's name, code, message) as JSON, and diff them against the same
-// battery run through this module. When core changes these message
-// formats, such a test is what catches the drift.
+// test/test-validate-parity.js proves the parity by diffing this module
+// against the real internals under --expose-internals.
 
 const { inspect } = require('node:util');
 
@@ -220,4 +214,5 @@ module.exports = {
   validateInteger,
   invalidArgTypeError,
   invalidArgValueError,
+  outOfRangeError,
 };

test/common/validate-battery.js

@@ -0,0 +1,110 @@
+'use strict';
+
+// The input battery for test-validate-parity.js. Both sides share
+// this module, so the compared inputs can never drift apart.
+
+// A JSON-safe, human-readable label for a call.
+function label(fn, args) {
+  const rendered = args.map((a) =>
+    typeof a === 'string' ? `'${a}'` : String(a),
+  );
+  return `${fn}(${rendered.join(', ')})`;
+}
+
+// Calls fn with args and describes the outcome: nothing thrown, or the
+// thrown error's name, code, and message.
+function outcome(fn, args) {
+  try {
+    fn(...args);
+    return { thrown: false };
+  } catch (err) {
+    return {
+      thrown: true,
+      name: err.name,
+      code: err.code,
+      message: err.message,
+    };
+  }
+}
+
+const kValidatorCases = [
+  ['validateString', [123, 'x']],
+  ['validateString', [undefined, 'x']],
+  ['validateString', [null, 'x']],
+  ['validateString', [true, 'x']],
+  ['validateString', [Symbol('s'), 'x']],
+  ['validateString', [{ a: 1 }, 'x']],
+  ['validateString', [['a'], 'x']],
+  ['validateString', [() => {}, 'x']],
+  ['validateString', ['ok', 'x']],
+  ['validateObject', [null, 'x']],
+  ['validateObject', [[1, 2], 'x']],
+  ['validateObject', ['str', 'x']],
+  ['validateObject', [123, 'x']],
+  ['validateObject', [() => {}, 'x']],
+  ['validateObject', [{}, 'x']],
+  ['validateInteger', [0, 'x', 1]],
+  ['validateInteger', ['abc', 'x', 1]],
+  ['validateInteger', [1.5, 'x', 1]],
+  ['validateInteger', [NaN, 'x', 1]],
+  ['validateInteger', [5, 'x', 1]],
+  ['validateInteger', [9007199254740999, 'x', 1]],
+];
+
+// Error-constructor cases: impl.fn(...args) must return the error object.
+const kErrorCases = [
+  [
+    'invalidArgTypeError',
+    ['options.filter', ['string', 'null', 'undefined'], 42],
+  ],
+  ['invalidArgTypeError', ['options.endpoint', ['string'], 123]],
+  ['invalidArgTypeError', ['options.maxBufferSize', ['integer'], 'abc']],
+  ['invalidArgTypeError', ['options.foo', ['Object'], []]],
+  [
+    'invalidArgTypeError',
+    ['first argument', ['string', 'null', 'undefined'], 42],
+  ],
+  [
+    'invalidArgValueError',
+    ['options.endpoint', 'not-a-valid-url', 'must be a valid URL'],
+  ],
+  [
+    'invalidArgValueError',
+    ['options.serviceName', '', 'must be a non-empty string'],
+  ],
+  ['invalidArgValueError', ['options.maxBufferSize', 0]],
+  ['outOfRangeError', ['x', '>= 1 && <= 100', 0]],
+  ['outOfRangeError', ['n', 'an integer', 10000000000]],
+];
+
+// Runs the battery through impl: either this package's lib/validate.js
+// exports or wrappers around core's internal error constructors.
+function runBattery(impl) {
+  const results = [];
+
+  for (const [name, args] of kValidatorCases) {
+    results.push({
+      call: label(name, args),
+      outcome: outcome(impl[name], args),
+    });
+  }
+
+  for (const [name, args] of kErrorCases) {
+    const err = impl[name](...args);
+    results.push({
+      call: label(name, args),
+      outcome: {
+        thrown: true,
+        name: err.name,
+        code: err.code,
+        message: err.message,
+      },
+    });
+  }
+
+  return results;
+}
+
+module.exports = {
+  runBattery,
+};

test/fixtures/parity-child.js

@@ -0,0 +1,29 @@
+'use strict';
+
+// Spawned with --expose-internals and the battery module in
+// BATTERY_MODULE: runs the battery through core's real internals and
+// prints the outcomes as JSON for the parent to compare.
+
+const {
+  validateString,
+  validateObject,
+  validateInteger,
+} = require('internal/validators');
+const {
+  codes: { ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_OUT_OF_RANGE },
+} = require('internal/errors');
+const { runBattery } = require(process.env.BATTERY_MODULE);
+
+const impl = {
+  validateString,
+  validateObject,
+  validateInteger,
+  invalidArgTypeError: (name, expected, actual) =>
+    new ERR_INVALID_ARG_TYPE(name, expected, actual),
+  invalidArgValueError: (name, value, reason) =>
+    new ERR_INVALID_ARG_VALUE(name, value, reason),
+  outOfRangeError: (str, range, input) =>
+    new ERR_OUT_OF_RANGE(str, range, input),
+};
+
+process.stdout.write(JSON.stringify(runBattery(impl)));

test/test-exporter.js

@@ -3,14 +3,53 @@
 const assert = require('node:assert');
 const http = require('node:http');
 const { describe, it } = require('node:test');
-const { fixture, spawnMicrotel } = require('./common/otel');
+const { flush } = require('../lib/flush');
+const {
+  fixture,
+  httpGet,
+  startHTTPServer,
+  spawnMicrotel,
+} = require('./common/otel');
 
 // This test verifies exporter error handling and process lifecycle behavior.
 // Tracing is activated in the child processes through the public init() API;
 // the children run fixtures from test/fixtures with JSON init options on
 // argv.
 
 describe('otel exporter behavior', () => {
+  it('warns when the collector responds non-2xx', async () => {
+    // An in-process collector that rejects every export with a 500. The
+    // spans are dropped, and a throttled OTelExportWarning reports it.
+    const collector = http.createServer((req, res) => {
+      res.writeHead(500);
+      res.end();
+    });
+    await new Promise((resolve) => collector.listen(0, '127.0.0.1', resolve));
+
+    const warningPromise = new Promise((resolve) => {
+      process.on('warning', function onWarning(w) {
+        if (w.name === 'OTelExportWarning') {
+          process.removeListener('warning', onWarning);
+          resolve(w);
+        }
+      });
+    });
+
+    const { start } = require('../lib/core');
+    start({ endpoint: `http://127.0.0.1:${collector.address().port}` });
+
+    const target = await startHTTPServer();
+    await httpGet(target.port, '/x');
+    flush();
+
+    const warning = await warningPromise;
+
+    await collector.close();
+    await target.close();
+
+    assert.match(warning.message, /HTTP 500/);
+  });
+
   it('does not crash when collector is unreachable', async () => {
     // The child traces a request against its own server and flushes to a
     // dead endpoint. The export fails, but the child exits normally.

test/test-flush-coverage.js

@@ -5,7 +5,7 @@ const { after, afterEach, before, describe, it } = require('node:test');
 
 const { Span, SPAN_KIND_INTERNAL } = require('../lib/span');
 const { addSpan, flush, resetCaches } = require('../lib/flush');
-const { startTracing, delay } = require('./common/otel');
+const { startTracing, getSpans, delay } = require('./common/otel');
 
 describe('flush.js coverage', () => {
   let collector;
@@ -22,12 +22,51 @@ describe('flush.js coverage', () => {
   });
 
   it('flush is a no-op when buffer is empty', async () => {
+    const before = collector.requests.length;
+
     flush();
 
     // Wait for any potential export request.
     await delay(100);
 
-    assert.strictEqual(collector.requests.length, 0);
+    assert.strictEqual(collector.requests.length, before);
+  });
+
+  it('encodes boolean, double, and fallback attribute values', async () => {
+    // A span-like object covering the OTLP encodings the instrumentations
+    // never produce: booleans, non-integer numbers, and values of other
+    // types, which fall back to strings.
+    const span = {
+      name: 'mixed-values',
+      traceId: 'a'.repeat(32),
+      spanId: 'b'.repeat(16),
+      kind: 1,
+      startTime: 100,
+      endTime: 200,
+      getAttributes() {
+        return {
+          bool: true,
+          ratio: 1.5,
+          misc: null,
+        };
+      },
+      getEvents() {
+        return [];
+      },
+      status: { code: 0, message: '' },
+    };
+
+    addSpan(span);
+    flush();
+
+    const payload = await collector.next();
+    const attrs = {};
+    for (const a of getSpans(payload)[0].attributes) {
+      attrs[a.key] = a.value;
+    }
+    assert.deepStrictEqual(attrs.bool, { boolValue: true });
+    assert.deepStrictEqual(attrs.ratio, { doubleValue: 1.5 });
+    assert.deepStrictEqual(attrs.misc, { stringValue: 'null' });
   });
 
   it('flush handles spanToOtlp errors gracefully', async () => {
@@ -95,6 +134,8 @@ describe('flush.js coverage', () => {
   });
 
   it('resetCaches clears the span buffer', async () => {
+    const before = collector.requests.length;
+
     // A span so the buffer is non-empty.
     const span = new Span('test', SPAN_KIND_INTERNAL);
     span.end();
@@ -106,6 +147,6 @@ describe('flush.js coverage', () => {
     // Wait for any potential export request.
     await delay(100);
 
-    assert.strictEqual(collector.requests.length, 0);
+    assert.strictEqual(collector.requests.length, before);
   });
 });

test/test-http-client.js

@@ -218,6 +218,49 @@ describe('otel HTTP client spans', () => {
     );
   });
 
+  it('parses IPv6 authorities and default ports from the Host header', async () => {
+    const target = await startHTTPServer((req, res) => {
+      res.writeHead(200);
+      res.end('ok');
+    });
+
+    const seen = [];
+    const request = (headers, path) =>
+      new Promise((resolve, reject) => {
+        const req = http.get(
+          { host: '127.0.0.1', port: target.port, path, headers },
+          (res) => {
+            res.resume();
+            res.on('end', resolve);
+          },
+        );
+        req.on('error', reject);
+      });
+
+    // A bracketed IPv6 authority: the port follows the closing bracket.
+    await request({ Host: `[::1]:9999` }, '/ipv6');
+    // A portless Host header means the scheme default.
+    await request({ Host: 'example.test' }, '/default-port');
+
+    const spans = await flushAndCollect(collector);
+    await target.close();
+
+    const v6 = getClientSpan(spans, '/ipv6');
+    assert.ok(v6, 'Expected a client span for the IPv6 request');
+    assert.partialDeepStrictEqual(spanAttrs(v6), {
+      'server.port': '9999',
+      'url.full': 'http://[::1]:9999/ipv6',
+    });
+
+    const def = getClientSpan(spans, '/default-port');
+    assert.ok(def, 'Expected a client span for the portless Host request');
+    assert.strictEqual(
+      spanAttrs(def)['server.port'],
+      '80',
+      'A portless Host header must yield the scheme default',
+    );
+  });
+
   it('creates error span for HTTP client connection refused', async () => {
     await httpGet(1, '/fail').catch(() => {});