ronantakizawa/codereview-bench huggingface dataset:
Repository: nodejs/node
File: tools/dep_updaters/update-test426-fixtures.sh (Shell)

Code before:
#!/bin/sh

set -e

BASE_DIR=$(cd "$(dirname "$0")/../.." && pwd)

TARGET_DIR="$BASE_DIR/test/fixtures/test426"
README="$BASE_DIR/test/test426/README.md"
TARBALL_URL=$(curl -fsIo /dev/null -w '%header{Location}' https://github.com/tc39/source-map-tests/archive/HEAD.tar.gz)
SHA=$(basename "$TARBALL_URL")

TMP_DIR="$(mktemp -d)"

rm -rf "$TARGET_DIR"
mkdir "$TARGET_DIR"
curl -f "$TARBALL_URL" | tar -xz --strip-components 1 -C "$TARGET_DIR"

rm -rf "$TMP_DIR"

TMP_FILE=$(mktemp)
sed "s#https://github.com/tc39/source-map-tests/commit/[0-9a-f]*#https://github.com/tc39/source-map-tests/commit/$SHA#" "$README" > "$TMP_FILE"
mv "$TMP_FILE" "$README"

echo "test426 fixtures updated to $SHA."


Review comment (suggestion):
```suggestion
TMP_FILE=$(mktemp)
sed "s/$CURRENT_SHA/$SHA/" "$README" > "$TMP_FILE"
```

Code after:
#!/bin/sh

set -ex

BASE_DIR=$(cd "$(dirname "$0")/../.." && pwd)

TARGET_DIR="$BASE_DIR/test/fixtures/test426"
README="$BASE_DIR/test/test426/README.md"
TARBALL_URL=$(curl -fsIo /dev/null -w '%header{Location}' https://github.com/tc39/source-map-tests/archive/HEAD.tar.gz)
SHA=$(basename "$TARBALL_URL")

CURRENT_SHA=$(sed -n 's#^.*https://github.com/tc39/source-map-tests/commit/\([0-9a-f]*\).*$#\1#p' "$README")

if [ "$CURRENT_SHA" = "$SHA" ]; then
  echo "Already up-to-date"
  exit 0
fi

rm -rf "$TARGET_DIR"
mkdir "$TARGET_DIR"
curl -f "$TARBALL_URL" | tar -xz --strip-components 1 -C "$TARGET_DIR"

TMP_FILE=$(mktemp)
sed "s/$CURRENT_SHA/$SHA/" "$README" > "$TMP_FILE"
mv "$TMP_FILE" "$README"

# The last line of the script should always print the new version,
# as we need to add it to $GITHUB_ENV variable.
echo "NEW_VERSION=$SHA"
Repository: nodejs/node
File: tools/dep_updaters/update-test426-fixtures.sh (Shell)

Code before:
#!/bin/sh

set -e

BASE_DIR=$(cd "$(dirname "$0")/../.." && pwd)

TARGET_DIR="$BASE_DIR/test/fixtures/test426"
README="$BASE_DIR/test/test426/README.md"
TARBALL_URL=$(curl -fsIo /dev/null -w '%header{Location}' https://github.com/tc39/source-map-tests/archive/HEAD.tar.gz)
SHA=$(basename "$TARBALL_URL")

TMP_DIR="$(mktemp -d)"

rm -rf "$TARGET_DIR"
mkdir "$TARGET_DIR"
curl -f "$TARBALL_URL" | tar -xz --strip-components 1 -C "$TARGET_DIR"

rm -rf "$TMP_DIR"

TMP_FILE=$(mktemp)
sed "s#https://github.com/tc39/source-map-tests/commit/[0-9a-f]*#https://github.com/tc39/source-map-tests/commit/$SHA#" "$README" > "$TMP_FILE"
mv "$TMP_FILE" "$README"

echo "test426 fixtures updated to $SHA."


Review comment (bug):
We get a much more useful debug output with `set -x`
```suggestion
set -ex
```

Code after:
#!/bin/sh

set -ex

BASE_DIR=$(cd "$(dirname "$0")/../.." && pwd)

TARGET_DIR="$BASE_DIR/test/fixtures/test426"
README="$BASE_DIR/test/test426/README.md"
TARBALL_URL=$(curl -fsIo /dev/null -w '%header{Location}' https://github.com/tc39/source-map-tests/archive/HEAD.tar.gz)
SHA=$(basename "$TARBALL_URL")

CURRENT_SHA=$(sed -n 's#^.*https://github.com/tc39/source-map-tests/commit/\([0-9a-f]*\).*$#\1#p' "$README")

if [ "$CURRENT_SHA" = "$SHA" ]; then
  echo "Already up-to-date"
  exit 0
fi

rm -rf "$TARGET_DIR"
mkdir "$TARGET_DIR"
curl -f "$TARBALL_URL" | tar -xz --strip-components 1 -C "$TARGET_DIR"

TMP_FILE=$(mktemp)
sed "s/$CURRENT_SHA/$SHA/" "$README" > "$TMP_FILE"
mv "$TMP_FILE" "$README"

# The last line of the script should always print the new version,
# as we need to add it to $GITHUB_ENV variable.
Repository: nodejs/node
File: test/parallel/test-util-inspect-proxy.js (JavaScript)

Code before:
const proxy9 = new Proxy(Date, String);
const expected8 = 'Proxy [ [Function: Date], [] ]';
const expected9 = 'Proxy [ [Function: Date], [Function: String] ]';
assert.strictEqual(util.inspect(proxy8, opts), expected8);
assert.strictEqual(util.inspect(proxy9, opts), expected9);
assert.strictEqual(util.inspect(proxy8), '[Function: Date]');
assert.strictEqual(util.inspect(proxy9), '[Function: Date]');

const proxy10 = new Proxy(() => {}, {});
const proxy11 = new Proxy(() => {}, {
  get() {
    return proxy11;
  },
  apply() {
    return proxy11;
  }
});
const expected10 = '[Function (anonymous)]';
const expected11 = '[Function (anonymous)]';
assert.strictEqual(util.inspect(proxy10), expected10);
assert.strictEqual(util.inspect(proxy11), expected11);

{
  // Nested proxies should not trigger any proxy handlers.
  const errorProxy = new Proxy({}, { get(...cause) { throw new EvalError('boom', { cause }); } });
  const nestedProxy = new Proxy(errorProxy, {});

  util.inspect(nestedProxy, { showProxy: true });
  util.inspect(nestedProxy, { showProxy: false });
}


Review comment (suggestion):
We can reuse throwing handler from above
```suggestion
  const nestedProxy = new Proxy(proxyObj, handler);
```

Code after:
// Just for fun, let's create a Proxy using Arrays.
const proxy7 = new Proxy([], []);
const expected7 = 'Proxy [ [], [] ]';
assert.strictEqual(util.inspect(proxy7, opts), expected7);
assert.strictEqual(util.inspect(proxy7), 'Proxy([])');

// Now we're just getting silly, right?
const proxy8 = new Proxy(Date, []);
const proxy9 = new Proxy(Date, String);
const expected8 = 'Proxy [ [Function: Date], [] ]';
const expected9 = 'Proxy [ [Function: Date], [Function: String] ]';
assert.strictEqual(util.inspect(proxy8, opts), expected8);
assert.strictEqual(util.inspect(proxy9, opts), expected9);
assert.strictEqual(util.inspect(proxy8), 'Proxy([Function: Date])');
assert.strictEqual(util.inspect(proxy9), 'Proxy([Function: Date])');

const proxy10 = new Proxy(() => {}, {});
const proxy11 = new Proxy(() => {}, {
  get() {
    return proxy11;
  },
  apply() {
    return proxy11;
  }
});
const expected10 = 'Proxy([Function (anonymous)])';
const expected11 = 'Proxy([Function (anonymous)])';
assert.strictEqual(util.inspect(proxy10), expected10);
assert.strictEqual(util.inspect(proxy11), expected11);

const proxy12 = new Proxy([1, 2, 3], proxy5);
assert.strictEqual(
  util.inspect(proxy12, { colors: true, breakLength: 1 }),
  '\x1B[36mProxy(\x1B[39m' +
    '[\n  \x1B[33m1\x1B[39m,\n  \x1B[33m2\x1B[39m,\n  \x1B[33m3\x1B[39m\n]\x1B[36m' +
    ')\x1B[39m'
);
assert.strictEqual(util.format('%s', proxy12), 'Proxy([ 1, 2, 3 ])');

{
  // Nested proxies should not trigger any proxy handlers.
  const nestedProxy = new Proxy(new Proxy(new Proxy({}, handler), {}), {});

  assert.strictEqual(
    util.inspect(nestedProxy, { showProxy: true }),
    'Proxy [ Proxy [ Proxy [ {}, [Object] ], {} ], {} ]'
  );
  assert.strictEqual(util.inspect(nestedProxy, { showProxy: false }), expected3NoShowProxy);
}

{
Repository: nodejs/node
File: test/parallel/test-parse-args.mjs (JavaScript)

Code before:
import { spawnPromisified } from '../common/index.mjs';
import assert from 'node:assert';
import { suite, test } from 'node:test';
import { parseArgs } from 'node:util';

test('when short option used as flag then stored as flag', () => {
  const args = ['-f'];
  const expected = { values: { __proto__: null, f: true }, positionals: [] };
  const result = parseArgs({ strict: false, args });
  assert.deepStrictEqual(result, expected);
});

test('when short option used as flag before positional then stored as flag and positional (and not value)', () => {
  const args = ['-f', 'bar'];
  const expected = { values: { __proto__: null, f: true }, positionals: [ 'bar' ] };
  const result = parseArgs({ strict: false, args });
  assert.deepStrictEqual(result, expected);
});

test('when short option `type: "string"` used with value then stored as value', () => {
  const args = ['-f', 'bar'];
  const options = { f: { type: 'string' } };
  const expected = { values: { __proto__: null, f: 'bar' }, positionals: [] };
  const result = parseArgs({ args, options });
  assert.deepStrictEqual(result, expected);
});


Review comment (suggestion):
nit: `path` is usually referring to `node:path`, it's confusing to have it refer to a function
```suggestion
import * as fixtures from '../common/fixtures.mjs';
```

EDIT: I guess it should be a follow up along with a lint rule

Code after:
import { spawnPromisified } from '../common/index.mjs';
import { path } from '../common/fixtures.mjs';
import assert from 'node:assert';
import { suite, test } from 'node:test';
import { parseArgs } from 'node:util';

test('when short option used as flag then stored as flag', () => {
  const args = ['-f'];
  const expected = { values: { __proto__: null, f: true }, positionals: [] };
  const result = parseArgs({ strict: false, args });
  assert.deepStrictEqual(result, expected);
});

test('when short option used as flag before positional then stored as flag and positional (and not value)', () => {
  const args = ['-f', 'bar'];
  const expected = { values: { __proto__: null, f: true }, positionals: [ 'bar' ] };
  const result = parseArgs({ strict: false, args });
  assert.deepStrictEqual(result, expected);
});

test('when short option `type: "string"` used with value then stored as value', () => {
  const args = ['-f', 'bar'];
  const options = { f: { type: 'string' } };
  const expected = { values: { __proto__: null, f: 'bar' }, positionals: [] };
  const result = parseArgs({ args, options });
  assert.deepStrictEqual(result, expected);
});
Repository: nodejs/node
File: test/parallel/test-eventtarget-custom-inspect-does-not-throw.js (JavaScript)

Code before:
'use strict';

const assert = require('assert');
const util = require('util');

const symbol = util.inspect.custom;

const eventTargetInspect = EventTarget.prototype[symbol];

const fakeEventTarget = {
  [symbol]: eventTargetInspect,
  someOtherField: 42
};

assert.doesNotThrow(() => {
  util.inspect(fakeEventTarget);
});


Review comment (suggestion):
```suggestion
util.inspect(fakeEventTarget);
```

Using `assert.doesNotThrow()` is discouraged and is prohibited by the linter.

Ideally, we can verify the output as well.

Code after:
'use strict';

require('../common');
const assert = require('assert');
const util = require('util');

const symbol = util.inspect.custom;

const eventTargetInspect = EventTarget.prototype[symbol];

const fakeEventTarget = {
  [symbol]: eventTargetInspect,
  someOtherField: 42
};

// Should not throw when calling the custom inspect method
const output = util.inspect(fakeEventTarget);

assert.strictEqual(typeof output, 'string');
Repository: nodejs/node
File: test/parallel/test-fs-promises-file-handle-dispose.js (JavaScript)

Code before:
const { opendirSync, promises: fs } = require('fs');

async function explicitCall() {
  const fh = await fs.open(__filename);
  fh.on('close', common.mustCall());
  await fh[Symbol.asyncDispose]();

  const dh = await fs.opendir(__dirname);
  await dh[Symbol.asyncDispose]();
  // Repeat invocations should not reject
  await dh[Symbol.asyncDispose]();
  await assert.rejects(dh.read(), { code: 'ERR_DIR_CLOSED' });

  const dhSync = opendirSync(__dirname);
  dhSync[Symbol.dispose]();
  // Repeat invocations should not throw
  dhSync[Symbol.dispose]();
  assert.throws(() => dhSync.readSync(), { code: 'ERR_DIR_CLOSED' });
}

async function implicitCall() {
  {
    await using fh = await fs.open(__filename);
    fh.on('close', common.mustCall());
  }

  let dh;
  {
    await using dirHandle = await fs.opendir(__dirname);
    dh = dirHandle;
  }
  await assert.rejects(dh.read(), { code: 'ERR_DIR_CLOSED' });

  let dhSync;
  {
    using dirHandleSync = opendirSync(__dirname);
    dhSync = dirHandleSync;
  }
  assert.throws(() => dhSync.readSync(), { code: 'ERR_DIR_CLOSED' });
}

explicitCall().then(common.mustCall());
implicitCall().then(common.mustCall());


Review comment (suggestion):
This ensures only that `fh` is closed *at some point*, but we need to test that the filehandle is closed as soon as we exit the scope.

Code after:
const { opendirSync, promises: fs } = require('fs');

async function explicitCall() {
  const fh = await fs.open(__filename);
  fh.on('close', common.mustCall());
  await fh[Symbol.asyncDispose]();

  const dh = await fs.opendir(__dirname);
  await dh[Symbol.asyncDispose]();
  // Repeat invocations should not reject
  await dh[Symbol.asyncDispose]();
  await assert.rejects(dh.read(), { code: 'ERR_DIR_CLOSED' });

  const dhSync = opendirSync(__dirname);
  dhSync[Symbol.dispose]();
  // Repeat invocations should not throw
  dhSync[Symbol.dispose]();
  assert.throws(() => dhSync.readSync(), { code: 'ERR_DIR_CLOSED' });
}

async function implicitCall() {
  let fh;
  {
    await using openHandle = await fs.open(__filename);
    fh = openHandle;
    fh.on('close', common.mustCall());
  }
  assert.strictEqual(fh.fd, -1);

  let dh;
  {
    await using dirHandle = await fs.opendir(__dirname);
    dh = dirHandle;
  }
  await assert.rejects(dh.read(), { code: 'ERR_DIR_CLOSED' });

  let dhSync;
  {
    using dirHandleSync = opendirSync(__dirname);
    dhSync = dirHandleSync;
  }
  assert.throws(() => dhSync.readSync(), { code: 'ERR_DIR_CLOSED' });
}

explicitCall().then(common.mustCall());
implicitCall().then(common.mustCall());
Repository: nodejs/node
File: test/parallel/test-fs-promises-statfs-validate-path.js (JavaScript)

Code before:
'use strict';

const common = require('../common');
const fs = require('fs');
const assert = require('assert');

(async () => {
  await assert.rejects(
    fs.promises.statfs(),
    (err) => {
      assert.strictEqual(err.code, 'ERR_INVALID_ARG_TYPE');
      return true;
    }
  );
})().then(common.mustCall());


Review comment (suggestion):
This would give more meaningful error and stack trace in case of regression:
```suggestion
    { code: 'ERR_INVALID_ARG_TYPE' },
```

Code after:
'use strict';

const common = require('../common');
const fs = require('fs');
const assert = require('assert');

(async () => {
  await assert.rejects(
    fs.promises.statfs(),
    { code: 'ERR_INVALID_ARG_TYPE' },
  );
})().then(common.mustCall());
Repository: nodejs/node
File: lib/internal/fs/glob.js (JavaScript)

Code before:

/**
 * @param {string} path
 * @returns {Promise<DirentFromStats|null>}
 */
async function getDirent(path) {
  let stat;
  try {
    stat = await lstat(path);
  } catch {
    return null;
  }
  return new DirentFromStats(basename(path), stat, dirname(path));
}

/**
 * @param {string} path
 * @returns {DirentFromStats|null}
 */
function getDirentSync(path) {
  try {
    const stat = lstatSync(path, { throwIfNoEntry: false });
    if (stat === undefined) {
      return null;
    }
    return new DirentFromStats(basename(path), stat, dirname(path));
  } catch {
    return null;
  }
}

/**
 * @callback validateStringArrayOrFunction
 * @param {*} value
 * @param {string} name
 */
const validateStringArrayOrFunction = hideStackFrames((value, name) => {
  if (ArrayIsArray(value)) {
    for (let i = 0; i < value.length; ++i) {
      if (typeof value[i] !== 'string') {
        throw new ERR_INVALID_ARG_TYPE(`${name}[${i}]`, 'string', value[i]);
      }
    }
    return;
  }
  if (typeof value !== 'function') {
    throw new ERR_INVALID_ARG_TYPE(name, ['string[]', 'function'], value);
  }
});

/**

Review comment (suggestion):
Can be consolidated to:

```suggestion
  let stat;
  try {
    stat = lstatSync(path);
  } catch {
    return null;
  }
  return new DirentFromStats(basename(path), stat, dirname(path));
```

Code after:

/**
 * @param {string} path
 * @returns {Promise<DirentFromStats|null>}
 */
async function getDirent(path) {
  let stat;
  try {
    stat = await lstat(path);
  } catch {
    return null;
  }
  return new DirentFromStats(basename(path), stat, dirname(path));
}

/**
 * @param {string} path
 * @returns {DirentFromStats|null}
 */
function getDirentSync(path) {
  let stat;
  try {
    stat = lstatSync(path);
  } catch {
    return null;
  }
  return new DirentFromStats(basename(path), stat, dirname(path));
}

/**
 * @callback validateStringArrayOrFunction
 * @param {*} value
 * @param {string} name
 */
const validateStringArrayOrFunction = hideStackFrames((value, name) => {
  if (ArrayIsArray(value)) {
    for (let i = 0; i < value.length; ++i) {
      if (typeof value[i] !== 'string') {
        throw new ERR_INVALID_ARG_TYPE(`${name}[${i}]`, 'string', value[i]);
      }
    }
    return;
  }
  if (typeof value !== 'function') {
    throw new ERR_INVALID_ARG_TYPE(name, ['string[]', 'function'], value);
  }
});

/**
 * @param {string} pattern
 * @param {options} options
Repository: nodejs/node
File: src/node_http_parser.cc (C++)

Code before:
        memcpy(slab_, str_, size_);
        str_ = slab_;
        using_slab_ = true;
      } else {
        char* s = new char[size_];
        memcpy(s, str_, size_);
        str_ = s;
        on_heap_ = true;
      }
    }
  }

  void Reset() {
    if (on_heap_) {
      delete[] str_;
      on_heap_ = false;
    }
    using_slab_ = false;
    str_ = nullptr;
    size_ = 0;
  }

  void Update(const char* str, size_t size) {
    if (str_ == nullptr) {
      str_ = str;
    } else if (on_heap_ || using_slab_ || str_ + size_ != str) {
      const size_t total = size_ + size;

      if (!on_heap_ && total <= kSlabSize) {
        if (!using_slab_) {
          memcpy(slab_, str_, size_);
          using_slab_ = true;
        }
        memcpy(slab_ + size_, str, size);
        str_ = slab_;
      } else {
        char* s = new char[total];
        memcpy(s, str_, size_);
        memcpy(s + size_, str, size);
        if (on_heap_) delete[] str_;
        on_heap_ = true;
        using_slab_ = false;
        str_ = s;
      }
    }
    size_ += size;
  }

  Local<String> ToString(Environment* env) const {
    if (size_ != 0)
      return OneByteString(env->isolate(), str_, size_);

Review comment (performance):
I'm wondering if this was inteded to refer to something broader than what this PR does; I assume genuinely efficient allocation would involve allocating slab memory for a single HTTP message instead of for each `StringPtr`

Code after:
  }

  // Check if pointer is within this allocator's buffer.
  bool Contains(const char* ptr) const {
    return ptr >= buffer_ && ptr < buffer_ + kSlabSize;
  }

  // Reset allocator for new message.
  void Reset() { length_ = 0; }

 private:
  char buffer_[kSlabSize];
  size_t length_ = 0;
};

struct StringPtr {
  StringPtr() = default;
  ~StringPtr() { Reset(); }

  StringPtr(const StringPtr&) = delete;
  StringPtr& operator=(const StringPtr&) = delete;

  // If str_ does not point to owned storage yet, this function makes it do
  // so. This is called at the end of each http_parser_execute() so as not
  // to leak references. See issue #2438 and test-http-parser-bad-ref.js.
  void Save(StringPtrAllocator* allocator) {
    if (str_ == nullptr || on_heap_ ||
        (allocator != nullptr && allocator->Contains(str_))) {
      return;
    }
    // Try allocator first, fall back to heap
    if (allocator != nullptr) {
      char* ptr = allocator->TryAllocate(size_);
      if (ptr != nullptr) {
        memcpy(ptr, str_, size_);
        str_ = ptr;
        return;
      }
    }
    char* s = new char[size_];
    memcpy(s, str_, size_);
    str_ = s;
    on_heap_ = true;
  }

  void Reset() {
    if (on_heap_) {
      delete[] str_;
      on_heap_ = false;
    }
    str_ = nullptr;
Repository: nodejs/node
File: src/node_http_parser.cc (C++)

Code before:
 public:
  BindingData(Realm* realm, Local<Object> obj) : BaseObject(realm, obj) {}

  SET_BINDING_ID(http_parser_binding_data)

  std::vector<char> parser_buffer;
  bool parser_buffer_in_use = false;

  void MemoryInfo(MemoryTracker* tracker) const override {
    tracker->TrackField("parser_buffer", parser_buffer);
  }
  SET_SELF_SIZE(BindingData)
  SET_MEMORY_INFO_NAME(BindingData)
};

class Parser;

class StringPtrAllocator {
 public:
  // Memory impact: ~8KB per parser (66 StringPtr × 128 bytes).
  static constexpr size_t kSlabSize = 8192;

  StringPtrAllocator() { buffer_.SetLength(0); }

  // Allocate memory from the slab. Returns nullptr if full.
  char* Allocate(size_t size) {
    const size_t current = buffer_.length();
    if (current + size > kSlabSize) {
      return nullptr;
    }
    buffer_.SetLength(current + size);
    return buffer_.out() + current;
  }

  // Check if pointer is within this allocator's buffer.
  bool Contains(const char* ptr) const {
    return ptr >= buffer_.out() && ptr < buffer_.out() + buffer_.capacity();
  }
  // Reset allocator for new message.
  void Reset() { buffer_.SetLength(0); }

 private:
  MaybeStackBuffer<char, kSlabSize> buffer_;
};

struct StringPtr {
  StringPtr() = default;
  ~StringPtr() { Reset(); }

  StringPtr(const StringPtr&) = delete;
  StringPtr& operator=(const StringPtr&) = delete;

Review comment (bug):
Can we call this something like `TryAllocate()` so that it's obvious that this can fail to allocate even in non-OOM situations?

Code after:
 public:
  BindingData(Realm* realm, Local<Object> obj) : BaseObject(realm, obj) {}

  SET_BINDING_ID(http_parser_binding_data)

  std::vector<char> parser_buffer;
  bool parser_buffer_in_use = false;

  void MemoryInfo(MemoryTracker* tracker) const override {
    tracker->TrackField("parser_buffer", parser_buffer);
  }
  SET_SELF_SIZE(BindingData)
  SET_MEMORY_INFO_NAME(BindingData)
};

class Parser;

class StringPtrAllocator {
 public:
  // Memory impact: ~8KB per parser (66 StringPtr × 128 bytes).
  static constexpr size_t kSlabSize = 8192;

  StringPtrAllocator() = default;

  // Allocate memory from the slab. Returns nullptr if full.
  char* TryAllocate(size_t size) {
    if (length_ + size > kSlabSize) {
      return nullptr;
    }
    char* ptr = buffer_ + length_;
    length_ += size;
    return ptr;
  }

  // Check if pointer is within this allocator's buffer.
  bool Contains(const char* ptr) const {
    return ptr >= buffer_ && ptr < buffer_ + kSlabSize;
  }

  // Reset allocator for new message.
  void Reset() { length_ = 0; }

 private:
  char buffer_[kSlabSize];
  size_t length_ = 0;
};

struct StringPtr {
  StringPtr() = default;
  ~StringPtr() { Reset(); }
Repository: nodejs/node
File: lib/internal/test_runner/test.js (JavaScript)

Code before:
    return this.#test.name;
  }

  get filePath() {
    return this.#test.entryFile;
  }

  get fullName() {
    return getFullName(this.#test);
  }

  get error() {
    return this.#test.error;
  }

  get passed() {
    return this.#test.passed;
  }

  get attempt() {
    return this.#test.attempt ?? 0;
  }

  get workerId() {
    const envWorkerId = process.env.NODE_TEST_WORKER_ID;
    return envWorkerId !== undefined ? Number(envWorkerId) : undefined;
  }

  diagnostic(message) {
    this.#test.diagnostic(message);
  }

  plan(count, options = kEmptyObject) {
    if (this.#test.plan !== null) {
      throw new ERR_TEST_FAILURE(
        'cannot set plan more than once',
        kTestCodeFailure,
      );
    }

    this.#test.plan = new TestPlan(count, options);
  }

  get assert() {
    if (this.#assert === undefined) {
      const { plan } = this.#test;
      const map = lazyAssertObject(this.#test.root.harness);
      const assert = { __proto__: null };

      this.#assert = assert;
      map.forEach((method, name) => {

Review comment (refactor):
I think you can simplify this:


```suggestion
    return Number(envWorkerId) || undefined;
```

`Number(undefined)` → `NaN` (falsy)

`envWorkerId` should never be zero, so I that should be safe.

Code after:
  get name() {
    return this.#test.name;
  }

  get filePath() {
    return this.#test.entryFile;
  }

  get fullName() {
    return getFullName(this.#test);
  }

  get error() {
    return this.#test.error;
  }

  get passed() {
    return this.#test.passed;
  }

  get attempt() {
    return this.#test.attempt ?? 0;
  }

  get workerId() {
    const envWorkerId = process.env.NODE_TEST_WORKER_ID;
    return Number(envWorkerId) || undefined;
  }

  diagnostic(message) {
    this.#test.diagnostic(message);
  }

  plan(count, options = kEmptyObject) {
    if (this.#test.plan !== null) {
      throw new ERR_TEST_FAILURE(
        'cannot set plan more than once',
        kTestCodeFailure,
      );
    }

    this.#test.plan = new TestPlan(count, options);
  }

  get assert() {
    if (this.#assert === undefined) {
      const { plan } = this.#test;
      const map = lazyAssertObject(this.#test.root.harness);
      const assert = { __proto__: null };

      this.#assert = assert;