Cleaned up ghost/core db (integration) test suite noise (#28909)

no ref

Removes the bulk of the error-level log noise from the DB-backed test
suites (integration / e2e), where deliberate error paths were logging
hundreds of lines per run and burying real failures.

Author: 9larsons

ghost/core/core/server/services/email-address/email-address-service.ts

@@ -101,8 +101,19 @@ export class EmailAddressService {
      */
     getAddress(preferred: EmailAddresses, options: GetAddressOptions = {useFallbackAddress: false}): EmailAddresses {
         if (preferred.replyTo && !this.#isValidEmailAddress(preferred.replyTo.address)) {
+            // The default from-address itself is allowed to fail generic email validation
+            // (e.g. noreply@127.0.0.1 on an IP-only install — see the same carve-out in
+            // validate() below), so don't error-log when the replyTo just happens to match
+            // it (case-insensitively — members_support_address is user-entered, so a case
+            // variant of the same address shouldn't defeat the carve-out). This also
+            // silences a genuinely malformed mail:from, not just the known IP-domain case —
+            // demote to warn instead of dropping the signal entirely.
+            if (preferred.replyTo.address.toLowerCase() !== this.defaultFromEmail.address.toLowerCase()) {
+                logging.error(`[EmailAddresses] Invalid replyTo address: ${preferred.replyTo.address}`);
+            } else {
+                logging.warn(`[EmailAddresses] replyTo address ${preferred.replyTo.address} matches the default from-address and failed generic validation; removing it`);
+            }
             // Remove invalid replyTo addresses
-            logging.error(`[EmailAddresses] Invalid replyTo address: ${preferred.replyTo.address}`);
             preferred.replyTo = undefined;
         }
 
@@ -153,8 +164,10 @@ export class EmailAddressService {
                 return preferred;
             }
 
-            // Invalid configuration: don't allow to send from this sending domain
-            logging.error(`[EmailAddresses] Invalid configuration: cannot send emails from ${preferred.from.address} when sending domain is ${this.sendingDomain}`);
+            // Not an error: this is the expected, handled fallback for any sender address that
+            // doesn't match the sending domain (staff/member/newsletter senders routinely don't) —
+            // we fall through to the default from-address below. Warn-level, not error-level.
+            logging.warn(`[EmailAddresses] Cannot send emails from ${preferred.from.address} when sending domain is ${this.sendingDomain} — falling back to the default from-address`);
         }
 
         // Only allow to send from the configured from address

ghost/core/core/server/services/explore-ping/index.js

@@ -25,6 +25,13 @@ module.exports.createService = function createService() {
 };
 
 module.exports.init = async function init() {
+    // The explore ping is a background "phone home" request. It should not run
+    // in the test environment (cf. the update-check service, which gates on the
+    // same environments), where there is no explore URL configured.
+    if (!config.isProductionOrDevelopment()) {
+        return;
+    }
+
     const explorePingService = module.exports.createService();
 
     // The final intention is to have this run on a schedule

ghost/core/core/server/services/stripe/service.js

@@ -18,7 +18,7 @@ const settingsCache = require('../../../shared/settings-cache');
 async function configureApi() {
     const cfg = getConfig({settingsHelpers, config, urlUtils});
     if (cfg) {
-        cfg.testEnv = process.env.NODE_ENV.startsWith('test');
+        cfg.testEnv = config.isTestEnv();
         await module.exports.configure(cfg);
         return true;
     }

ghost/core/core/server/services/stripe/stripe-service.js

@@ -206,11 +206,23 @@ module.exports = class StripeService {
             webhookSecret: config.webhookSecret,
             webhookHandlerUrl: config.webhookHandlerUrl
         });
-        await this.webhookManager.start();
 
         this.billingPortalManager.configure({
             siteUrl: config.siteUrl
         });
-        await this.billingPortalManager.start();
+
+        // webhookManager.start() already self-guards: configure() above puts it in
+        // 'local' mode whenever a webhookSecret is set, which is always true outside
+        // production (config.js defaults it to DEFAULT_WEBHOOK_SECRET), so start()
+        // returns immediately without touching Stripe. Only billingPortalManager
+        // needs an explicit test-env skip — in the test env there is no real Stripe
+        // to register against, and the mock returns 500 for billing_portal/
+        // configurations, so this network-registration call only error-logs on
+        // every boot. Tests never need a registered portal configuration. Skip it
+        // under test; prod and dev register exactly as before.
+        await this.webhookManager.start();
+        if (!config.testEnv) {
+            await this.billingPortalManager.start();
+        }
     }
 };

ghost/core/core/server/services/update-check/index.js

@@ -1,6 +1,4 @@
 
-const _ = require('lodash');
-
 const api = require('../../api').endpoints;
 const config = require('../../../shared/config');
 const urlUtils = require('../../../shared/url-utils');
@@ -25,10 +23,8 @@ module.exports = async ({
     updateCheckUrl = config.get('updateCheck:url')
 } = {}) => {
     if (!forceUpdate) {
-        const allowedCheckEnvironments = ['development', 'production'];
-
-        // CASE: The check will not happen if your NODE_ENV is not in the allowed defined environments
-        if (_.indexOf(allowedCheckEnvironments, process.env.NODE_ENV) === -1) {
+        // CASE: The check will not happen if your env is not in the allowed defined environments
+        if (!config.isProductionOrDevelopment()) {
             return;
         }
     }

ghost/core/core/server/web/parent/middleware/log-request.js

@@ -1,4 +1,5 @@
 const logging = require('@tryghost/logging');
+const config = require('../../../../shared/config');
 
 /**
  * @TODO: move this middleware to Framework monorepo?
@@ -15,7 +16,17 @@ module.exports = function logRequest(req, res, next) {
         req.userId = req.user ? (req.user.id ? req.user.id : req.user) : null;
 
         if (req.err && req.err.statusCode !== 404) {
-            logging.error({req: req, res: res, err: req.err});
+            // 4xx are client errors (validation, auth, rate limit), not server
+            // faults. Production keeps them at error (they're monitored); the test
+            // env sets logging:logClientErrorsAsError=false to demote them to warn,
+            // where every deliberate error-response assertion (expectStatus(4xx))
+            // would otherwise log a redundant line.
+            const isClientError = req.err.statusCode >= 400 && req.err.statusCode < 500;
+            if (isClientError && config.get('logging:logClientErrorsAsError') === false) {
+                logging.warn({req: req, res: res, err: req.err});
+            } else {
+                logging.error({req: req, res: res, err: req.err});
+            }
         } else {
             logging.info({req: req, res: res});
         }

ghost/core/core/shared/config/defaults.json

@@ -72,6 +72,7 @@
     },
     "logging": {
         "level": "info",
+        "logClientErrorsAsError": true,
         "useLocalTime": false,
         "rotation": {
             "enabled": false,

ghost/core/core/shared/config/env/config.testing-mysql.json

@@ -16,7 +16,8 @@
         }
     },
     "logging": {
-        "level": "error"
+        "level": "error",
+        "logClientErrorsAsError": false
     },
     "adapters": {
         "Redis": {

ghost/core/core/shared/config/env/config.testing.json

@@ -12,7 +12,8 @@
         "port": 2369
     },
     "logging": {
-        "level": "error"
+        "level": "error",
+        "logClientErrorsAsError": false
     },
     "adapters": {
         "Redis": {

ghost/core/core/shared/config/helpers.js

@@ -97,14 +97,42 @@ const getContentPath = function getContentPath(type) {
     }
 };
 
+/**
+ * @callback isTestEnvFn
+ * @returns {boolean}
+ */
+const isTestEnv = function isTestEnv() {
+    return this.get('env').startsWith('test');
+};
+
+/**
+ * env defaults to 'development' when NODE_ENV is unset (see getNodeEnv() in
+ * ./utils.js), matching ghost.js's own `process.env.NODE_ENV = process.env.NODE_ENV
+ * || 'development'` at the real CLI entry point — so in any Ghost boot via the
+ * normal entry point, this and a raw `process.env.NODE_ENV` check agree. They
+ * only diverge for programmatic embedders that require core modules directly
+ * without going through ghost.js and never set NODE_ENV themselves — those now
+ * count as 'development' (e.g. explore-ping/update-check will phone home)
+ * rather than being silently skipped.
+ * @callback isProductionOrDevelopmentFn
+ * @returns {boolean}
+ */
+const isProductionOrDevelopment = function isProductionOrDevelopment() {
+    return ['development', 'production'].includes(this.get('env'));
+};
+
 /**
  * @typedef ConfigHelpers
  * @property {isPrivacyDisabledFn} isPrivacyDisabled
  * @property {getContentPathFn} getContentPath
+ * @property {isTestEnvFn} isTestEnv
+ * @property {isProductionOrDevelopmentFn} isProductionOrDevelopment
  */
 module.exports.bindAll = (nconf) => {
     nconf.isPrivacyDisabled = isPrivacyDisabled.bind(nconf);
     nconf.getContentPath = getContentPath.bind(nconf);
     nconf.getBackendMountPath = getBackendMountPath.bind(nconf);
     nconf.getFrontendMountPath = getFrontendMountPath.bind(nconf);
+    nconf.isTestEnv = isTestEnv.bind(nconf);
+    nconf.isProductionOrDevelopment = isProductionOrDevelopment.bind(nconf);
 };