Repository navigation
[PoC] Tests (2 of 2) - #181
Open
ajaynes-godaddy wants to merge 25 commits into
Open
ajaynes-godaddy wants to merge 25 commits into
ajaynes-godaddy wants to merge 25 commits into
Conversation
Importing lib/sake.js currently calls initConfig() at module load time, which exits the process if the cwd isn't a real plugin directory. This makes it impossible to import anything under tasks/ (all of which import sake) from a test runner outside a plugin fixture. Skipping initConfig() under SAKE_TEST_MODE lets test files import freely and call sake.initConfig() themselves once a fixture plugin directory is in place as cwd. Production CLI usage never sets this variable, so behavior there is unchanged.
Add test/fixtures/plugin/, a minimal real-looking WordPress plugin (main plugin file with a Plugin Name header, changelog.txt, a composer.json declaring the wc-plugin-framework dependency sake's own buildFrameworkConfig() expects) so sake.initConfig() can run against a realistic plugin directory without crashing. The composer.json is required: without a declared framework version, buildFrameworkConfig() calls semver.gte() on undefined and throws - this is a pre-existing gap in sake's own code for plugins that don't declare the framework dependency, surfaced by building this fixture. Add test/support/fixture-plugin.js, exporting enterFixturePlugin()/ exitFixturePlugin() to chdir into the fixture and set DEPLOY_DEV (the fixture has no .git directory, so sake can't otherwise infer a dev repo slug for buildDeployConfig()) and restore both afterward. Verified end to end: entering the fixture, running sake.initConfig(), and exiting all behave as expected.
Set SAKE_TEST_MODE in the script itself rather than inline in a test file, since ESM import hoisting would otherwise evaluate test file imports before an inline assignment takes effect. node --test test/ will report "no test files found" until the first *.test.js file lands - expected at this point, resolved by the next commit.
Scope npm test's glob to 'test/**/*.test.js' instead of the bare test/ directory. Node's default test-file discovery treats any .js file under a directory named test/ as a candidate test file, which would otherwise sweep up test/support/fixture-plugin.js (a helper module, not a test) and, in later phases, non-test fixture .js files. Add test/_phase0-smoke.test.js, a throwaway test proving every tasks/*.js file imports cleanly under SAKE_TEST_MODE, both from the repo root and from the fixture plugin directory. To be deleted once Phase 1 covers the same import surface with real assertions.
Cover hasArg, shouldFix, shouldSkipLinting, shouldFailOnLintErrors, and shouldShowFiles against process.argv, including a check that unrelated flags don't produce false positives on other checks.
Both were previously nested closures inside gitHubCreateReleaseMilestonesTask and gitHubCreateMonthMilestonesTask respectively. Neither references anything from its enclosing task function besides the module-level dateFormat import, so moving them to module scope and exporting them is behavior-preserving - the task functions still call them exactly as before. Add test/tasks/github.test.js covering both: tuesday count/day-of-week for a given year, MM/DD naming, string-vs-number year input, monthly milestone count, end-of-month dating, and full month/year naming.
Both were already module-level functions in tasks/prompt.js, just not
exported - purely additive change, no behavior change.
Cover filterIncrement's custom/skip/semver.inc branches, and getDefault's
mapping from sake.getDefaultIncrement() to a default prompt choice
index, mocking getDefaultIncrement() via node:test's t.mock.method to
exercise getDefault's own switch logic directly. (Noted but out of
scope here: getDefaultIncrement() itself always returns 'patch' in
practice - config.plugin.changes.join('\n', '* Feature ') passes a
second argument to Array.join that gets silently ignored, so the
intended '* Feature ' needle is never actually passed to
_str.include().)
Covers getPrereleaseVersions, getPluginName, normalizePath, resolvePath, tailingSlashPath, getVersionBump, getDefaultIncrement, parseChangelog, and getMainPluginFile, using the fixture plugin directory for the config-dependent ones. Documents two pre-existing behavior quirks found while writing these rather than silently working around them: getPrereleaseVersions returns an empty array for every realistic input (its only real call site passes a dash-free version, and even the standard semver.inc prerelease format defeats its loop's numeric coercion), and getDefaultIncrement always returns 'patch' for the same Array.join argument bug noted in the previous commit.
Add two fixture JS files under test/fixtures/eslint-samples/: a clean one and a broken one with a deliberate missing-semicolon error and an eqeqeq warning, matching the bundled default ESLint config's rules (including its space-in-parens requirement). Cover the clean/broken count reporting, string-path vs glob-array input, the no-matching-files early return, and the failOnErrors throw path.
Covers all seven del()-based tasks (cleanDevTask, cleanComposerTask, cleanBuildTask, cleanWcRepoTask, cleanPrereleaseTask, cleanWpTrunkTask, cleanWpAssetsTask) against real scratch directories, confirming targeted files/dirs are removed and unrelated ones are left alone. Mutates sake.config's existing keys in place for save/restore between tests rather than reassigning sake.config to a new object - every exported method closes over the original config object directly, so reassigning the sake.config property silently decouples it from a replacement object.
Extract the ~150-line glob-array construction (exclusions for the framework, composer dev packages, wp assets, custom excludes, etc.) into its own exported buildCopyBuildPaths() function, called by copyBuildTask exactly as before - the gulp.src/pipe/dest chain is otherwise unchanged, so this is a pure relocation with no behavior difference. Add test/tasks/copy.test.js covering buildCopyBuildPaths across the framework/non-framework, wc/wp deploy, composer require-dev, and custom-exclude branches, plus real scratch-directory tests for copyPrereleaseTask, copyWcRepoTask, copyWpTrunkTask, copyWpAssetsTask, and copyWpTagTask.
Covers zipTask's filename (plugin id + current version), default and custom-override zip destination resolution, and that it leaves other zips already in the build directory untouched. buildAndZip is a bare gulp.series(buildTask, zipTask) composition with no inspectable metadata and no logic of its own - calling it would run the full build pipeline, so coverage here is limited to what's verifiable without invoking it (it's a function, with the expected display name).
Covers bumpTask's version bump in the main plugin file and includes/Plugin.php (v5-framework plugins only), bumpMinReqsTask's WP/WC min and tested-up-to replacements, and bumpFrameworkVersionTask's FRAMEWORK_VERSION replacement. Documents two bugs found while writing these rather than silently working around them: replaceMinimumPhpVersion in pipes/replace.js never reflects options.minimum_php_version (it builds its replacement as a plain string once at module-import time instead of a function like every sibling pipe), and bumpFrameworkVersionTask's vendor- framework exclude pattern lacks a /** suffix, so it doesn't actually exclude files inside that directory.
Covers validateReadmeHeadersTask's required-header checks (License, License URI, and Stable tag for wp-deploy plugins) against fixture readme.txt content. sake.throwError() throws synchronously from inside an fs.readFile callback, so by the time it fires the task function has already returned - the throw surfaces as an uncaughtException rather than a normal catchable error, and node:test's own test harness unconditionally fails any test during which an uncaughtException occurs, with no way to mark it as an expected outcome. Mocking throwError to record the message instead of throwing sidesteps this and more directly asserts which header was reported missing.
Covers configTask's full-config logging, --property-filtered logging via dottie.get, and the undefined-property case, mocking console.log to capture what was logged.
processBundle was already a plain module-level function, just not exported - purely additive change, no behavior change. Covers the empty/missing bundle array early-return, a successful copy (including destination folder creation), and the missing-package and copy-error branches against a real scratch node_modules fixture. bundleScriptsTask/bundleStylesTask are covered via a real successful delegation to processBundle rather than mocking it, since both call the local function directly rather than through the module's exports. Documents a callback-contract bug found while writing these: for a bundle array with multiple failing items, processBundle calls its done callback once per failure plus once more unconditionally after the loop, since forEach can't break early. Not fixed here - out of scope.
…asks - Export getGithub so tests can mock Octokit instance methods directly - Add coverage for gitHubGetReleaseIssueTask, gitHubGetWcIssuesTask, gitHubCreateDocsIssueTask, gitHubCreateReleaseTask, gitHubCreateReleaseMilestonesTask, and gitHubCreateMonthMilestonesTask
Add coverage for wcValidateTask, wcUploadTask, wcDeployTask, and formatError, mocking axios.post against the WooCommerce.com submission runner API.
…Task - Add shell.test.js, mocking shell.exec for the ~15 git/svn/composer command-building tasks in tasks/shell.js - Add makepot.test.js, mocking shell.exec for the wp i18n make-pot invocation - Add upfw.test.js for setFrameworkVersionTask and updateFrameworkTask - Add bundleTask coverage to the existing bundle.test.js, mocking shell.exec for its npm install step
Add coverage for promptDeployTask, promptWcUploadTask, and promptTestedReleaseZipTask, mocking inquirer.prompt to run each non-interactively.
Runs on ubuntu-latest with actions SHA-pinned to node24-native releases (checkout v7.0.1, setup-node v7.0.0), resolving the Node version from package.json's existing engines field rather than hardcoding one.
GitHub Actions auto-annotates ESLint's default output format wherever it appears in a job's log, so the two tests that run runESLint against the deliberately broken fixture file were showing up as PR annotations pointing at a test fixture, not a real lint issue in the repo.
3 tasks done
ajaynes-godaddy
added a commit
that referenced
this pull request
Oct 4, 2026
Covers the logic introduced for MWC-1505: isNonInteractive()/isDryRunDeploy()/ newPluginVersion() in helpers/arguments.js, the --skip-linting gating in compile(), the --dry-run/--non-interactive gating in deployTask(), the --new-version fallback in replaceVersionTask(), and the non-interactive short-circuits in promptDeployTask/promptWcUploadTask/promptTestedReleaseZipTask. Builds on the test infrastructure merged in from PR #182 rather than reinventing it; does not re-cover the pre-existing functions/behavior already tested by PR #181.
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.
Summary
Adds
node:test-based test coverage forsake's core library and task modules. This PR depends on #182, which adds the test infrastructure (theSAKE_TEST_MODEgate, fixture plugin directory, npm scripts, and CI workflow) this coverage runs against — this PR is based on that branch and contains only the test files themselves, plus a handful of small additive exports needed to make specific functions reachable from a test.Details
No new dependencies were added. Tests run against the fixture plugin directory added in #182, and mock filesystem, network, and shell-command interactions as needed.
Test coverage by area
Line coverage is from Node's built-in
--experimental-test-coveragereporter, measured per source file.helpers/arguments.jslib/sake.jstasks/bump.jstasks/bundle.jstasks/clean.jstasks/config.jstasks/copy.jstasks/deploy.jstasks/github.jstasks/lint.jstasks/makepot.jstasks/prompt.jstasks/shell.jstasks/upfw.jstasks/validate.jstasks/wc.jstasks/zip.jsQA
npm testpasses locallyBefore merge
masteronce it does)