Skip to content

Removed test:unit scripts from projects with no Vitest unit tests#28116

Merged
9larsons merged 2 commits into
mainfrom
chore/drop-noop-test-unit-scripts
May 25, 2026
Merged

Removed test:unit scripts from projects with no Vitest unit tests#28116
9larsons merged 2 commits into
mainfrom
chore/drop-noop-test-unit-scripts

Conversation

@9larsons
Copy link
Copy Markdown
Contributor

@9larsons 9larsons commented May 25, 2026

Three workspaces had a test:unit script that didn't actually run unit tests, which inflated the ci_test_unit matrix from 13 real test runs to 16 and made the intent of those projects' test wiring hard to read.

  • apps/signup-form: test:unit was pnpm build — running the build, not tests. There was a single test/unit/hello.test.js (0 bytes) that never executed under that script. Removed the script and the placeholder file.
  • apps/sodo-search: test:unit chained through test:ci to vitest run --coverage against zero unit test files. The project's actual tests are Playwright (test/e2e/*.test.ts) and run in the separate unit_tests_playwright matrix — unaffected by this change. Removed both test:unit and test:ci (the latter has no other callers).
  • ghost-admin: test:unit was true (no-op). ghost-admin's tests run via ember exam under its own test target, invoked by the dedicated ghost-admin:test CI job. Removed.

After this change, pnpm nx run-many -t test:unit runs 13 projects instead of 16. Nx skips projects without the target — no extra wiring or exemption list to maintain. If/when one of these workspaces adds real Vitest tests, adding the test:unit script back is a one-line change.

Versions bumped on signup-form and sodo-search per the app-version-bump check. ghost-admin is private: true and not in .github/scripts/check-app-version-bump.js's MONITORED_APPS, so no bump needed.

- apps/signup-form's test:unit ran 'pnpm build' (not tests). Its only file in test/unit/ was an empty hello.test.js that never executed. Removed both.
- apps/sodo-search's test:unit ran vitest run --coverage against zero unit test files. Its Playwright tests live under test:acceptance and are unaffected. Removed test:unit and the test:ci script it chained through.
- ghost-admin's test:unit was 'true' (no-op). ghost-admin's actual tests run via 'ember exam' under its own test target, invoked by the dedicated ghost-admin CI job.

nx run-many -t test:unit now skips these three projects (16 → 13). Adding tests later means adding the script back — one line — and nx picks them up automatically.

Bumped signup-form and sodo-search versions for the app-version-bump check; ghost-admin is not in MONITORED_APPS so no bump needed.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2d27049e-1287-4316-9996-0c3ef047f680

📥 Commits

Reviewing files that changed from the base of the PR and between ae81e90 and 27a1c60.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

Walkthrough

This PR increments patch versions and removes deprecated npm test scripts across three app manifests, and updates the CI workflow to detect unit-test-global changes and compute a dedicated unit_test_projects_str. Package changes: apps/signup-form → 0.3.27 (removed test:unit), apps/sodo-search → 1.8.21 (removed test:ci and test:unit), ghost/admin (removed test:unit). CI changes: add paths-filter entries for unit-test globals, compute and expose job_setup.outputs.unit_test_projects_str, and gate/run the unit-test job using that value.

Possibly related PRs

  • TryGhost/Ghost#28038: Modifies CI unit-test execution logic around test:unit project selection (overlaps workflow changes).
  • TryGhost/Ghost#27901: Adds an additional test step in the unit-tests job and adjusts job_unit-tests logic (related CI job changes).
  • TryGhost/Ghost#28030: Changes ghost/core test:unit behavior to Vitest, which interacts with CI selection and test:unit targeting.

Suggested reviewers

  • rob-ghost
  • EvanHahn
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: removing test:unit scripts from three projects that lack actual Vitest unit tests, which is the core focus of this changeset.
Description check ✅ Passed The description thoroughly explains why each test:unit script was removed, what they were running, and the impact on CI, clearly relating to the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/drop-noop-test-unit-scripts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented May 25, 2026

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit ae81e90

Command Status Duration Result
nx run-many -t test:unit -p @tryghost/signup-fo... ✅ Succeeded <1s View ↗

☁️ Nx Cloud last updated this comment at 2026-05-25 19:49:45 UTC

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented May 25, 2026

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit ae81e90

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m View ↗
nx run @tryghost/admin-x-settings:test:acceptance ✅ Succeeded 9m 18s View ↗
nx run ghost:test:ci:e2e ✅ Succeeded 7m 39s View ↗
nx run ghost:test:ci:legacy ✅ Succeeded 3m 3s View ↗
nx build @tryghost/comments-ui ✅ Succeeded <1s View ↗
nx build @tryghost/portal ✅ Succeeded <1s View ↗
nx build @tryghost/announcement-bar ✅ Succeeded <1s View ↗
nx build @tryghost/signup-form ✅ Succeeded <1s View ↗
Additional runs (12) ✅ Succeeded ... View ↗

☁️ Nx Cloud last updated this comment at 2026-05-25 20:18:11 UTC

ref #28115

Shared Vitest config changes can affect more tests than Nx marks as changed, so CI now widens unit-test project selection for root config changes while keeping Ghost core config changes scoped to Ghost core.
@9larsons 9larsons enabled auto-merge (squash) May 25, 2026 20:14
@codecov
Copy link
Copy Markdown

codecov Bot commented May 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.83%. Comparing base (dd6b011) to head (27a1c60).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #28116   +/-   ##
=======================================
  Coverage   73.83%   73.83%           
=======================================
  Files        1528     1528           
  Lines      129467   129454   -13     
  Branches    15518    15518           
=======================================
- Hits        95587    95580    -7     
+ Misses      32919    32889   -30     
- Partials      961      985   +24     
Flag Coverage Δ
e2e-tests 76.13% <ø> (+2.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@9larsons 9larsons merged commit 8a34b4b into main May 25, 2026
49 checks passed
@9larsons 9larsons deleted the chore/drop-noop-test-unit-scripts branch May 25, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant