Add Apply Command and Testing#44
Open
mark-robustelli wants to merge 60 commits into
Open
Conversation
making obvious fixes from co-pilot review. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Each test file previously exported a single function wrapping all describe/test blocks, requiring a manual aggregator to call them. Test blocks are now at the top level so node:test can register them on import without any orchestration. - Remove export function wrappers from postInstall/index.js, telemetry/index.js, validator.test.js, variable-substitution.test.js, and apply.integration.test.js - Promote mockedTrueConfig/mockedFalseConfig to module scope in telemetry - Promote fusionAuthUrl/apiKey to module scope in apply integration test - Remove unused imports (json, config, randomUUID) from postInstall/index.js - Update test.js to use side-effect imports instead of named imports and explicit function calls
Removes the custom test.js orchestrator and replaces it with dedicated npm scripts that use node --test directly, making package.json the single source of truth for which files each test suite runs. - Delete __tests__/test.js aggregator - Add test, test:unit, and test:integration npm scripts in package.json - Remove RUN_INTEGRATION_TESTS and SKIP_UNIT_TESTS env vars from all configs (replaced by running the appropriate npm script) - Update .github/workflows/integration-tests.yml to use npm run test:integration - Update __tests__/integration/README.md to document the three new scripts
Renames two test files that used index.js as their entry point to follow the same *.test.js naming convention used by the rest of the test suite. - Rename __tests__/postInstall/index.js -> __tests__/postInstall/postinstall.test.js - Rename __tests__/telemetry/index.js -> __tests__/telemetry/telemetry.test.js - Update test and test:unit scripts in package.json to reference new file names
…tput Replaces all dist/ imports with src/ imports and adds the ts-node/esm loader to test scripts, eliminating the need to build before running tests. - Update all test files to import from src/ instead of dist/ - Add --no-warnings=ExperimentalWarning --loader ts-node/esm to all test scripts - Remove preLaunchTask: build from test launch configurations - Update telemetry test mocks from dist/.fa to src/.fa (ts-node resolves __dirname to src/ instead of dist/ when loading TypeScript source)
adjusted after action to ensure successful test
…ocking The Node.js test runner cancels tests when the JavaScript event loop appears drained. Using execSync for docker compose startup blocked the JS thread for ~40 seconds (while pulling images), causing the test runner to report ERR_TEST_FAILURE with 'Promise resolution is still pending but the event loop has already resolved'. Replaced all execSync calls in setup.js with promisified async exec (execAsync) so the event loop stays active during docker compose operations, keeping the integration test alive."
Copilot
AI
changed the title
Mcr/kickstart apply
fix(tests): replace execSync with async exec to prevent event loop blocking in integration tests
Jun 10, 2026
Migrates from the deprecated --loader flag to --import=tsx, which uses the stable Node.js module registration API. - Replace --no-warnings=ExperimentalWarning --loader ts-node/esm with --import=tsx in start, test, test:integration, and test:unit scripts - Remove ts-node devDependency - Remove unused jest devDependency
Resolves deprecation warnings for actions running on Node.js 20. Node.js 20 actions will be forced to Node.js 24 on June 16, 2026.
…nally before() and after() called inside a test() body execute synchronously in declaration order, not as lifecycle hooks. This caused after() cleanup to run before the await logEvent() call, meaning nock interceptors and env vars were torn down before the function under test was invoked. - Replace before/after with inline try/finally in all three logEvent tests - Fix process.env.FUSIONAUTH_TELEMETRY assigned boolean instead of string - Remove unused before/after imports from node:test
add test for Telemetry set to false
add try catch to clean up variable in case of error
- Switch all unit test files to node:assert/strict so assert.equal uses === instead of == by default
Several sync test callbacks declared (t) => but never referenced t. Replaced with () => across all four unit test files.
.env.test is a test artifact and should not be included in repo.
extractErrorDetails() assumes FusionAuth generalErrors is string[] and fieldErrors is Record<string, string[]>, but FusionAuth errors are typically arrays of objects with message/code. As written, this can log [object Object] or miss useful error text.
…N formats
The previous implementation had two bugs:
1. Array start detection required the property name and [ to appear on
the same line, producing empty line maps when they were split across
lines.
2. } was used as the end-of-array sentinel instead of ], causing the
array to never be detected as closed.
Replaced the single-pass loop with a state machine
(seekingProperty -> seekingArrayOpen -> inArray) and separate depth
counters for [ ] (array container) and { } (element boundaries).
Also added string literal tracking so structural characters inside
JSON string values do not affect depth counts.
mark-robustelli
changed the title
The directory traversal guard resolvedPath.startsWith(kickstartDirResolved) is vulnerable to prefix-matching (e.g. /tmp/kickstart matches /tmp/kickstart-evil). Use path.relative() (or append path.sep) to ensure the resolved file stays within the kickstart directory. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause
The
integration-testsCI job was failing with:The
__tests__/integration/setup.jsfile usedexecSyncto run docker compose commands. Whendocker compose up -dneeded to pull Docker images,execSyncblocked the entire Node.js JavaScript thread for ~40 seconds. During this blocking period, the Node.js test runner could not process any callbacks, and when the thread finally unblocked, the test runner interpreted the lack of event loop activity as a drained event loop — cancelling the still-pending test promise.Changes Made
__tests__/integration/setup.js: Replaced allexecSynccalls withawait execAsync(usingpromisify(exec)fromnode:util) so the Node.js event loop stays active during docker compose operations:docker compose ps -q)docker compose down -v)docker compose --env-file .env.test up -d)stopFusionAuthContainer(docker compose down -v)Testing