Remove vows from tests. by jdmarshall · Pull Request #854 · node-config/node-config

package.json (1)

60-60: Switch to node:test runner via c8 node --test looks good

Using Node’s built-in test harness aligns with the PR objective and modernizes the test execution path.

test/17-toml.js (1)

16-27: LGTM: per-test setup and strict structural assertion

beforeEach reloading of config provides isolation, and deepStrictEqual is the right choice for object arrays here.

test/8-config-extending.js (1)

22-34: LGTM: behavior preserved while migrating off Vows

The test intent (“does not throw on deep extend after attachProtoDeep”) remains intact under node:test.

test/22-binary.js (1)

13-27: LGTM: immutability enforcement verified cleanly

The throws assertion correctly validates that binary config values are immutable.

test/23-serialize.js (1)

12-17: LGTM: Serialization smoke test is correct and resilient

Using assert.doesNotThrow around JSON.stringify(CONFIG.get('level1')) is appropriate here. The migration to node:test looks clean.

test/x-config-js-test-transpiled.js (1)

26-34: LGTM: Initialization and basic assertions look good

Clearing env, loading via requireUncached, and verifying CONFIG availability and expected title are sound.

test/21-date.js (1)

21-27: LGTM: Date method invocation guarded properly

Using assert.doesNotThrow to validate toISOString() on the loaded Date object is appropriate.

test/1-protected-test.js (16)

1-6: LGTM!

The migration to Node's built-in test runner is clean and follows standard patterns.


14-14: Good preservation of test state isolation

Storing the original argv to restore later is essential for preventing test interference. This approach correctly ensures test isolation.


25-50: Well-structured test initialization with proper isolation

The beforeEach hook properly resets environment variables and reloads the config module uncached, ensuring each test runs with a clean state. This is particularly important given the singleton nature of the config module.


52-56: LGTM - Clear and focused test

The assertion correctly validates the library is loaded as an object.


58-75: Comprehensive isObject() test coverage

Good coverage of edge cases including null, undefined, arrays, and primitives. The assertions correctly validate the behavior of the isObject utility.


77-144: Thorough deep cloning tests with proper edge case coverage

Excellent test coverage for the cloneDeep functionality including:

  • Deep equality checks
  • Reference vs value copying for arrays and objects
  • Special object preservation (Date, RegExp)
  • Mutation isolation verification

The tests properly validate that modifications to cloned objects don't affect originals.


146-224: Comprehensive extendDeep() test coverage

The test suite thoroughly validates deep merging behavior including:

  • Normal extension
  • Non-object replacement
  • Deep object merging
  • Date handling
  • Partial object creation when mixing types
  • Array type preservation
  • Prototype method preservation

All assertions correctly test the expected merge semantics.


226-296: Well-structured equalsDeep() tests

Good coverage of equality comparison scenarios including:

  • Empty objects
  • Arrays
  • Object property order independence
  • Deep nested objects
  • Null/undefined edge cases
  • Size mismatches
  • Value differences

The tests properly validate deep equality semantics.


298-340: Thorough diffDeep() validation

Tests properly validate diff generation including:

  • Empty diff for identical objects
  • Deep comparison ignoring property order
  • Shallow diff extraction
  • Deep nested diff extraction

The assertions correctly check both the structure and values of generated diffs.


342-592: Comprehensive substituteDeep() test suite with excellent error handling

This test suite provides extensive coverage of variable substitution including:

  • Empty mappings
  • Non-existent variables
  • JSON content handling
  • Edge cases with undefined, null, empty strings
  • Type validation with proper error assertions for invalid leaf types (Array, Boolean, Number, null, undefined, NaN)
  • Parser error handling with descriptive error messages

The error cases are particularly well-tested using assert.throws with proper validation.


594-641: Good setPath() test coverage

Tests properly validate path setting behavior including:

  • Null value handling (ignored)
  • Top-level key creation
  • Sub-key creation
  • Parent object creation
  • Value overwriting

All assertions correctly validate the expected behavior.


643-716: Comprehensive parseFile() tests across multiple formats

Excellent coverage of file parsing for YAML, CSON, and .properties formats. Tests validate:

  • Correct object structure
  • Property values
  • Array types
  • Variable replacements in .properties files
  • Section support

The assertions properly validate parsing behavior for each format.


718-738: Basic but sufficient loadFileConfigs() test

Tests validate that the function exists, returns an object, and loads configuration correctly.


740-784: Thorough attachProtoDeep() validation

Tests properly validate:

  • Function existence
  • Original object preservation
  • Prototype method attachment at all levels (including sub-objects)
  • Prototype methods are hidden from JSON serialization

The JSON serialization test is particularly important to ensure prototype pollution doesn't affect object iteration.


786-824: Well-structured getCmdLineArg() tests with proper cleanup

Tests comprehensively validate command-line argument parsing including:

  • Basic argument retrieval
  • Missing argument handling
  • Alternative syntax support
  • First-match precedence
  • Proper restoration of original argv

The cleanup in line 819-823 properly restores the original process.argv state.


826-854: Good toObject() serialization tests

Tests validate:

  • Function existence
  • POJO conversion (not an instance of Config)
  • Serialization of current instance
  • Serialization of provided arguments

The assertions correctly validate the serialization behavior.

test/x-config-test-ts-module-exports.js (3)

1-5: LGTM - Consistent test framework migration

The migration to Node's built-in test runner follows the same pattern as other test files in this PR.


16-32: Well-structured test setup with proper environment isolation

The beforeEach hook properly:

  • Clears environment variables from previous tests
  • Sets the TypeScript config directory
  • Disables strict mode
  • Loads config uncached for test isolation

This ensures clean state between tests when working with TypeScript config files using module.exports.


34-44: Clear and focused TypeScript configuration tests

The tests validate:

  1. Library availability as an object
  2. Correct loading of TypeScript configuration values

Simple but sufficient for validating TypeScript config support with module.exports.

test/19-custom-environment-variables.js (5)

1-7: LGTM - Consistent test framework migration

The migration follows the established pattern with proper imports.


8-27: Good test for unset environment variables

The test properly validates that configuration values remain unchanged when environment variables are not set.


29-47: Important edge case: empty string environment variables

Good coverage of the edge case where environment variables are set to empty strings. The test correctly validates that empty strings don't override configuration values.


49-71: Comprehensive test for environment variable overrides

Tests properly validate that:

  • String environment variables override config values
  • JSON environment variables are parsed correctly

The JSON parsing test is particularly important for complex configuration overrides.


73-97: Good test coverage for getCustomEnvVars() utility

The test validates the utility function for retrieving custom environment variable overrides from a specific directory. Properly sets up environment and validates both string and JSON values.

test/15-async-configs.js (4)

1-5: LGTM - Clean migration to node:test

The test file properly migrates to Node's built-in test runner.


24-30: Proper async test setup

The beforeEach correctly uses async/await to resolve async configurations before each test, ensuring the config is ready for assertions.


32-44: Comprehensive async config validation

Tests properly validate:

  • AsyncConfig values are resolved by resolveAsyncConfigs
  • Regular functions remain untouched
  • this context binding in async functions

Good coverage of the async configuration feature.


46-60: Thorough testing of complex async scenarios

Tests validate:

  • Objects returned from promises are treated as single values
  • Original values are preserved
  • Local promise values override originals
  • Deferred functionality works with AsyncConfig

Excellent coverage of edge cases in async configuration resolution.

test/22-files-order.js (3)

1-5: LGTM - Consistent test framework migration

The migration follows the established pattern across the test suite.


16-31: Well-structured test setup for file order validation

The beforeEach properly sets up the multi-file configuration scenario with:

  • Custom config directory
  • NODE_ENV=test
  • NODE_APP_INSTANCE=instance

This setup allows testing the correct loading order of configuration files.


33-37: Clear validation of configuration file precedence

The test correctly validates that configuration files are loaded in the expected order:

  • prop1 from default config
  • prop2 from JSON instance config (overrides)
  • prop3 from YML instance config (overrides)

This ensures the file precedence system works as intended.

test/10-gitcrypt-test.js (1)

7-38: LGTM: Migration and core assertions look solid

The rewrite to node:test is clear, with per-test isolation via requireUncached and correct positive/negative paths for CONFIG_SKIP_GITCRYPT. Assertions are precise.

test/7-unicode-situations.js (3)

1-1: Good addition of 'use strict'

This brings the test file in line with modern JavaScript best practices.


3-6: Successful migration to Node.js built-in test framework

The imports have been properly updated to use the Node.js built-in test framework (node:test). The migration from vows to the native test runner is clean and follows the consistent pattern used across the PR.


22-42: Well-structured test organization

The migration to nested describe blocks with focused it tests improves readability and follows standard testing conventions. The test logic remains intact while adopting the more modern structure.

test/0-util.js (1)

7-7: Clean migration to Node.js built-in test framework

The imports have been properly updated from vows to the Node.js built-in test framework. Good job maintaining consistency across the test suite migration.

test/20-multiple-config.js (2)

1-2: Good addition of 'use strict'

Adding strict mode improves code quality and helps catch potential errors.


5-6: Successful migration to Node.js built-in test framework

The migration from vows to Node.js built-in test runner is clean and consistent with the overall PR objectives.

test/x-deferred-configs-ts.js (3)

1-1: Good addition of 'use strict'

This aligns with best practices and the pattern used across all migrated test files.


5-7: Clean migration to Node.js built-in test framework

The imports are properly updated from vows to Node.js built-in test runner, maintaining consistency with the PR-wide migration.


22-53: Well-structured test migration

The test structure has been successfully migrated to use nested describe blocks with individual it tests. The test logic and assertions remain unchanged, preserving the original test coverage.

test/6-strict-mode.js (1)

1-1: Good addition of 'use strict'

Consistent with the modernization effort across all test files.

test/9-raw-configs.js (1)

13-37: Migration looks good and preserves behavior.

The switch to node:test, uncached load, and assertions for raw values look correct and consistent with the suite.

test/util.js (1)

77-91: Promise detection tests are solid.

Good coverage for created, resolved, and rejected promises (with catch to avoid unhandled rejections).

test/makeImmutable-shared-refs.js (2)

73-81: Idempotency test is appropriate and valuable.

Verifies no “Cannot redefine property” on the second call—good guard against regressions.


57-60: Assertion is correct — keep the existing "Can not" message

The code in lib/config.js throws the exact message using "Can not ... runtime configuration property" so the test's regex is valid.

  • lib/config.js:381-384 — throw Error(Can not ${message} runtime configuration property: "${name}". Configuration objects are immutable unless ALLOW_CONFIG_MUTATIONS is set.)
  • test/makeImmutable-shared-refs.js:57-60 — assertion uses /Can not update runtime configuration property/ and matches the thrown message.

No change required; ignore the original suggestion.

Likely an incorrect or invalid review comment.

test/18-skipConfigSources.js (1)

8-12: Conversion to node:test and structure LGTM.

The migration reads well and keeps the semantics of the original vows tests intact.

test/3-deferred-configs.js (1)

22-62: Migration preserves deferred-config semantics.

Good use of requireUncached and environment setup; the assertions map 1:1 with prior behavior.

test/5-getConfigSources.js (1)

8-35: Good migration from Vows to node:test with isolated setup per scenario.

Use of requireUncached and per-scenario beforeEach improves test isolation. Path.delimiter for multi-dir config is also a nice cross-platform touch.

test/2-config-test.js (1)

4-5: Nice migration to node:test and requireUncached harness.

Test structure is much clearer than the old Vows topics/batches and isolates lib loads well. Solid coverage retained across formats and features.