From 487d9b1c83e8f62d91ab26af247cb434952bb523 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Thu, 30 Jul 2026 16:08:37 -0500 Subject: [PATCH 1/3] fix(onboard-status): report audit/scrape state truthfully during onboarding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The onboarding status report conflated "still running" with "failed", so a status snapshot taken mid-onboarding wrongly showed most audits as having failed for the site. Root causes and fixes: - Audit run-status was derived by grepping CloudWatch audit-worker logs for a "Received audit request" line. Within the onboard wait window that line often hadn't landed yet, yielding a false " audit has not been executed". Replace this with the DB Audit records already loaded for the completion check (computeAuditCompletion): an audit that has not completed is reported as in progress (⏳), never as "not executed". Removes the fragile/slow CloudWatch dependency entirely (deletes cloudwatch-utils.js + its test and obsolete CloudWatch tests). - Scraping availability was binary (completedCount > 0 ? ✅ : ❌), so a scrape still in progress (0 complete, many URLs PENDING/RUNNING) showed as failed. Add a tri-state deriveScrapingStatus (available/in_progress/ failed/unknown) rendered as ✅/⏳/❌, and surface an "In progress: N" count in the scraping stats message. - security-vulnerabilities only runs for AEM_CS delivery type, so on aem_edge/other sites it can never produce an opportunity yet was reported as missing/failed. Add isOpportunityApplicableForDeliveryType and filter expectedOpportunityTypes by the site's delivery type. analyzeMissingOpportunities is now a pure function (DB-driven, no I/O) and is unit-tested directly, along with deriveScrapingStatus and the delivery-type predicate. Co-Authored-By: Claude Opus 4.8 --- .../opportunity-status-processor/handler.js | 217 ++++-- src/utils/cloudwatch-utils.js | 104 --- .../analyze-missing-opportunities.test.js | 101 +++ .../delivery-type-applicability.test.js | 35 + .../derive-scraping-status.test.js | 50 ++ .../opportunity-status-processor.test.js | 622 ++---------------- test/utils/cloudwatch-utils.test.js | 268 -------- 7 files changed, 391 insertions(+), 1006 deletions(-) delete mode 100644 src/utils/cloudwatch-utils.js create mode 100644 test/tasks/opportunity-status-processor/analyze-missing-opportunities.test.js create mode 100644 test/tasks/opportunity-status-processor/delivery-type-applicability.test.js create mode 100644 test/tasks/opportunity-status-processor/derive-scraping-status.test.js delete mode 100644 test/utils/cloudwatch-utils.test.js diff --git a/src/tasks/opportunity-status-processor/handler.js b/src/tasks/opportunity-status-processor/handler.js index 41843539..81f5eb31 100644 --- a/src/tasks/opportunity-status-processor/handler.js +++ b/src/tasks/opportunity-status-processor/handler.js @@ -22,12 +22,62 @@ import { getOpportunitiesForAudit, computeAuditCompletion, } from '@adobe/spacecat-shared-utils'; -import { getAuditStatus } from '../../utils/cloudwatch-utils.js'; import { checkAndAlertBotProtection } from '../../utils/bot-detection.js'; import { say } from '../../utils/slack-utils.js'; const TASK_TYPE = 'opportunity-status-processor'; +/** + * Derives a tri-state scraping status from aggregated scrape-URL counts so the report + * distinguishes "still running" from "failed". A snapshot taken mid-onboarding (many + * URLs still PENDING/RUNNING, none COMPLETE yet) must read as in-progress, not failed. + * + * @param {{completed: number, failed: number, pending: number, total: number}} [stats] + * @returns {'available'|'in_progress'|'failed'|'unknown'} + */ +export function deriveScrapingStatus(stats) { + if (!stats || stats.total === 0) { + return 'unknown'; + } + if (stats.completed > 0) { + return 'available'; + } + if (stats.pending > 0) { + return 'in_progress'; + } + return 'failed'; +} + +/** + * Opportunity types whose audit only runs for specific site delivery types. + * An opportunity absent from this map is applicable to every delivery type. + * Keeping this list conservative avoids hiding opportunities that could legitimately + * be produced — only encode audits with a hard delivery-type gate in the audit worker. + * + * - security-vulnerabilities: the audit skips unless delivery type is AEM_CS + * (spacecat-audit-worker src/vulnerabilities/handler.js), so on aem_edge/aem_ams/etc. + * it can never produce an opportunity and must not be reported as missing/failed. + */ +const OPPORTUNITY_DELIVERY_TYPE_RESTRICTIONS = { + 'security-vulnerabilities': ['aem_cs'], +}; + +/** + * Whether an opportunity type can be produced for a site of the given delivery type. + * Unrestricted opportunities (and unknown delivery types) are treated as applicable. + * + * @param {string} opportunityType + * @param {string} [deliveryType] - Site delivery type (e.g. 'aem_edge', 'aem_cs') + * @returns {boolean} + */ +export function isOpportunityApplicableForDeliveryType(opportunityType, deliveryType) { + const allowedDeliveryTypes = OPPORTUNITY_DELIVERY_TYPE_RESTRICTIONS[opportunityType]; + if (!allowedDeliveryTypes || !deliveryType) { + return true; + } + return allowedDeliveryTypes.includes(deliveryType); +} + /** * Checks if RUM is available for a domain by attempting to get a domainkey * @param {string} domain - The domain to check @@ -188,6 +238,10 @@ async function isScrapingAvailable(baseUrl, context, onboardStartTime) { // Count successful and failed scrapes across all jobs const completedCount = allUrlResults.filter((result) => result.status === 'COMPLETE').length; const failedCount = allUrlResults.filter((result) => result.status === 'FAILED').length; + // Non-terminal URLs (scrape still running) — used to distinguish in-progress from failed. + const pendingCount = allUrlResults.filter( + (result) => result.status === 'PENDING' || result.status === 'RUNNING', + ).length; const totalCount = allUrlResults.length; // Check if at least one URL was successfully scraped (status === 'COMPLETE') @@ -209,6 +263,7 @@ async function isScrapingAvailable(baseUrl, context, onboardStartTime) { stats: { completed: completedCount, failed: failedCount, + pending: pendingCount, total: totalCount, }, }; @@ -219,32 +274,35 @@ async function isScrapingAvailable(baseUrl, context, onboardStartTime) { } /** - * Analyzes missing opportunities and determines the root cause - * @param {Array} missingOpportunities - Array of missing opportunity types - * @param {Array} auditTypes - Array of audit types from profile - * @param {string} siteId - The site ID - * @param {number} onboardStartTime - The onboarding start timestamp - * @param {object} serviceStatus - Object containing status of all services - * @param {object} context - The context object - * @returns {Promise>} Analysis results + * Analyzes missing opportunities and determines the root cause. + * + * Pure function — derives audit execution state from the DB audit records + * (via `completedAuditTypes`, computed with `computeAuditCompletion`) instead of + * grepping CloudWatch logs. This removes the false "audit has not been executed" + * verdict that fired whenever a log line simply hadn't landed within the onboard + * wait window: an audit that has not completed yet is reported as *in progress*, + * not as a failure. + * + * @param {Array} missingOpportunities - Expected-but-missing opportunity types + * @param {Array} auditTypes - Audit types from the profile + * @param {Array} completedAuditTypes - Audit types with a fresh DB audit record + * @param {object} serviceStatus - Availability of each data source (rum/seoImport/scraping) + * @returns {Array<{opportunity: string, audit: string, reason: string, inProgress?: boolean}>} */ -async function analyzeMissingOpportunities( +export function analyzeMissingOpportunities( missingOpportunities, auditTypes, - siteId, - onboardStartTime, + completedAuditTypes, serviceStatus, - context, ) { const results = []; + const completed = new Set(completedAuditTypes || []); - /* eslint-disable no-await-in-loop */ for (const opportunityType of missingOpportunities) { // Find which audit(s) should generate this opportunity - const relatedAudits = auditTypes.filter((auditType) => { - const opportunities = getOpportunitiesForAudit(auditType); - return opportunities.includes(opportunityType); - }); + const relatedAudits = auditTypes.filter( + (auditType) => getOpportunitiesForAudit(auditType).includes(opportunityType), + ); if (relatedAudits.length === 0) { // eslint-disable-next-line no-continue @@ -252,19 +310,13 @@ async function analyzeMissingOpportunities( } for (const auditType of relatedAudits) { - // Get audit execution status and failure reason in a single call - const { executed, failureReason } = await getAuditStatus( - auditType, - siteId, - onboardStartTime, - context, - ); - - if (!executed) { + // Not completed yet → still running, not a failure. + if (!completed.has(auditType)) { results.push({ opportunity: opportunityType, audit: auditType, - reason: `${auditType} audit has not been executed`, + reason: `${auditType} audit is still in progress`, + inProgress: true, }); // eslint-disable-next-line no-continue continue; @@ -293,23 +345,14 @@ async function analyzeMissingOpportunities( continue; } - // All dependencies met, check for audit failure - if (failureReason) { - results.push({ - opportunity: opportunityType, - audit: auditType, - reason: `Audit failed: ${failureReason}`, - }); - } else { - results.push({ - opportunity: opportunityType, - audit: auditType, - reason: 'Audit executed successfully, found no issues to report (no opportunities created)', - }); - } + // Audit completed with all trackable dependencies met, but produced no opportunity. + results.push({ + opportunity: opportunityType, + audit: auditType, + reason: 'Audit executed successfully, found no issues to report (no opportunities created)', + }); } } - /* eslint-enable no-await-in-loop */ return results; } @@ -351,6 +394,7 @@ export async function runOpportunityStatusProcessor(message, context) { let seoImportAvailable = false; let gscConfigured = false; let scrapingAvailable = false; + let scrapingStats = null; const opportunities = await site.getOpportunities(); @@ -367,6 +411,13 @@ export async function runOpportunityStatusProcessor(message, context) { }); // Remove duplicates expectedOpportunityTypes = [...new Set(expectedOpportunityTypes)]; + // Drop opportunities whose audit cannot run for this site's delivery type + // (e.g. security-vulnerabilities on aem_edge), so they are not reported as + // missing/failed when they were never going to be produced. + const deliveryType = site.getDeliveryType(); + expectedOpportunityTypes = expectedOpportunityTypes.filter( + (oppType) => isOpportunityApplicableForDeliveryType(oppType, deliveryType), + ); } // Calculate which dependencies are needed based on expected opportunities @@ -420,6 +471,7 @@ export async function runOpportunityStatusProcessor(message, context) { if (needsScraping) { const scrapingCheck = await isScrapingAvailable(siteUrl, context, onboardStartTime); scrapingAvailable = scrapingCheck.available; + scrapingStats = scrapingCheck.stats || null; // Check for bot protection using all jobIds from scraping check // Multiple audit types create separate jobIds during onboarding @@ -444,11 +496,17 @@ export async function runOpportunityStatusProcessor(message, context) { // Scraping might still be running, so we show stats every time if (slackContext) { if (scrapingCheck.stats) { - const { completed, failed, total } = scrapingCheck.stats; + const { + completed, failed, pending = 0, total, + } = scrapingCheck.stats; + // Show in-progress count so a still-running scrape isn't misread as a + // failure (e.g. total 988 with only 159 failed + 829 still pending). + const pendingLine = pending > 0 ? `⏳ In progress: ${pending}\n` : ''; const statsMessage = `:mag: *Scraping Statistics for ${siteUrl}*\n` + `✅ Completed: ${completed}\n` - + `❌ Failed: ${failed}\n` - + `📊 Total: ${total}`; + + `❌ Failed: ${failed}\n${ + pendingLine + }📊 Total: ${total}`; if (failed > 0) { await say( @@ -489,6 +547,29 @@ export async function runOpportunityStatusProcessor(message, context) { scraping: scrapingAvailable, }; + // Determine which audits have completed vs are still pending, straight from the + // DB audit records (not CloudWatch logs). This single source drives both the + // in-progress (⏳) opportunity statuses and the missing-opportunity analysis, so a + // not-yet-completed audit is reported as in progress rather than "not executed". + // Only meaningful when we have an onboardStartTime anchor to compare against. + let pendingAuditTypes = []; + let completedAuditTypes = []; + if (auditTypes && auditTypes.length > 0 && onboardStartTime) { + try { + const { Audit } = dataAccess; + const latestAudits = await Audit.allLatestForSite(siteId); + const completion = computeAuditCompletion(auditTypes, onboardStartTime, latestAudits); + pendingAuditTypes = completion.pendingAuditTypes; + completedAuditTypes = completion.completedAuditTypes; + } catch (auditErr) { + log.warn(`Could not check audit completion from DB for site ${siteId}: ${auditErr.message}`); + // Conservative fallback: mark all as pending so nothing is misreported as + // failed/executed and the "may still be in progress" disclaimer is shown. + pendingAuditTypes = [...auditTypes]; + completedAuditTypes = []; + } + } + // Get actual opportunity types from site const actualOpportunityTypes = opportunities.map((opp) => opp.getType()); const uniqueActualOpportunityTypes = [...new Set(actualOpportunityTypes)]; @@ -505,13 +586,11 @@ export async function runOpportunityStatusProcessor(message, context) { // Analyze missing opportunities to determine root cause if (onboardStartTime) { - missingOpportunitiesAnalysis = await analyzeMissingOpportunities( + missingOpportunitiesAnalysis = analyzeMissingOpportunities( missingOpportunities, auditTypes, - siteId, - onboardStartTime, + completedAuditTypes, serviceStatus, - context, ); } } @@ -522,30 +601,20 @@ export async function runOpportunityStatusProcessor(message, context) { const rumStatus = rumAvailable ? ':white_check_mark:' : ':x:'; const seoImportStatus = seoImportAvailable ? ':white_check_mark:' : ':x:'; const gscStatus = gscConfigured ? ':white_check_mark:' : ':x:'; - const scrapingStatus = scrapingAvailable ? ':white_check_mark:' : ':x:'; + // Tri-state scraping: hourglass while URLs are still being scraped, so an + // in-progress snapshot is not mislabelled as a failure. + const scrapingStatusKey = deriveScrapingStatus(scrapingStats); + const scrapingEmoji = { + available: ':white_check_mark:', + in_progress: ':hourglass_flowing_sand:', + }[scrapingStatusKey] || ':x:'; + const scrapingStatus = scrapingEmoji; statusMessages.push(`RUM ${rumStatus}`); statusMessages.push(`SEO Import ${seoImportStatus}`); statusMessages.push(`GSC ${gscStatus}`); statusMessages.push(`Scraping ${scrapingStatus}`); - // Determine which audits are still pending so opportunity statuses can reflect - // in-progress state (⏳) rather than showing stale data as ✅/❌. - // Only meaningful when we have an onboardStartTime anchor to compare against. - let pendingAuditTypes = []; - if (auditTypes && auditTypes.length > 0 && onboardStartTime) { - try { - const { Audit } = dataAccess; - const latestAudits = await Audit.allLatestForSite(siteId); - const completion = computeAuditCompletion(auditTypes, onboardStartTime, latestAudits); - pendingAuditTypes = completion.pendingAuditTypes; - } catch (auditErr) { - log.warn(`Could not check audit completion from DB for site ${siteId}: ${auditErr.message}`); - // Conservative fallback: mark all as pending so disclaimer is always shown on error - pendingAuditTypes = [...auditTypes]; - } - } - // Process opportunities by type to avoid duplicates // Only process opportunities that are expected based on the profile's audit types const processedTypes = new Set(); @@ -613,7 +682,7 @@ export async function runOpportunityStatusProcessor(message, context) { dataSourceMessages.push(`GSC ${gscConfigured ? ':white_check_mark:' : ':x:'}`); } if (needsScraping) { - dataSourceMessages.push(`Scraping ${scrapingAvailable ? ':white_check_mark:' : ':x:'}`); + dataSourceMessages.push(`Scraping ${scrapingEmoji}`); } await say(env, log, slackContext, `*Data Sources for site ${siteUrl}*`); @@ -661,8 +730,14 @@ export async function runOpportunityStatusProcessor(message, context) { // Add missing opportunities analysis if (missingOpportunitiesAnalysis.length > 0) { for (const analysis of missingOpportunitiesAnalysis) { - // Use info icon for successful audits, error icon for actual failures - const emoji = analysis.reason.includes('found no issues to report') ? ':information_source:' : ':x:'; + // Hourglass for audits still running, info icon for audits that ran and + // found nothing, error icon only for actual failures (missing dependencies). + let emoji = ':x:'; + if (analysis.inProgress) { + emoji = ':hourglass_flowing_sand:'; + } else if (analysis.reason.includes('found no issues to report')) { + emoji = ':information_source:'; + } auditErrors.push(`*${analysis.opportunity}*: ${analysis.reason} ${emoji}`); } } diff --git a/src/utils/cloudwatch-utils.js b/src/utils/cloudwatch-utils.js deleted file mode 100644 index f8f6d735..00000000 --- a/src/utils/cloudwatch-utils.js +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -import { CloudWatchLogsClient, FilterLogEventsCommand } from '@aws-sdk/client-cloudwatch-logs'; - -const AUDIT_WORKER_LOG_GROUP = '/aws/lambda/spacecat-services--audit-worker'; - -/** - * Creates a CloudWatch Logs client - * @param {object} env - Environment variables - * @returns {CloudWatchLogsClient} Configured CloudWatch client - */ -function createCloudWatchClient(env) { - return new CloudWatchLogsClient({ - region: env.AWS_REGION || 'us-east-1', - }); -} - -/** - * Calculates the search window start time with buffer - * @param {number} onboardStartTime - The onboarding start timestamp (ms) - * @param {number} bufferMs - Buffer time in milliseconds (default: 5 minutes) - * @returns {number} The search start time in milliseconds - */ -function calculateSearchWindow(onboardStartTime, bufferMs = 5 * 60 * 1000) { - return onboardStartTime - ? onboardStartTime - bufferMs - : Date.now() - 30 * 60 * 1000; // 30 minutes ago as fallback -} - -/** - * Gets the execution status and failure reason for an audit by searching Audit Worker logs. - * This replaces the separate checkAuditExecution and getAuditFailureReason functions, - * reducing redundant CloudWatch API calls. - * - * @param {string} auditType - The audit type to search for - * @param {string} siteId - The site ID - * @param {number} onboardStartTime - The onboarding start timestamp - * @param {object} context - The context object with env and log - * @returns {Promise} Object with { executed: boolean, failureReason: string|null } - */ -export async function getAuditStatus(auditType, siteId, onboardStartTime, context) { - const { log, env } = context; - const logGroupName = env.AUDIT_WORKER_LOG_GROUP || AUDIT_WORKER_LOG_GROUP; - const cloudWatchClient = createCloudWatchClient(env); - - try { - // Check if audit was executed - const executionFilterPattern = `"Received ${auditType} audit request for: ${siteId}"`; - const searchStartTime = calculateSearchWindow(onboardStartTime, 5 * 60 * 1000); // 5 min buffer - - const executionCommand = new FilterLogEventsCommand({ - logGroupName, - filterPattern: executionFilterPattern, - startTime: searchStartTime, - endTime: Date.now(), - }); - - const executionResponse = await cloudWatchClient.send(executionCommand); - const executed = executionResponse.events && executionResponse.events.length > 0; - - if (!executed) { - return { executed: false, failureReason: null }; - } - - // Audit was executed, check for failure - const failureFilterPattern = `"${auditType} audit for ${siteId} failed"`; - const failureStartTime = calculateSearchWindow(onboardStartTime, 30 * 1000); // 30 sec buffer - - const failureCommand = new FilterLogEventsCommand({ - logGroupName, - filterPattern: failureFilterPattern, - startTime: failureStartTime, - endTime: Date.now(), - }); - - const failureResponse = await cloudWatchClient.send(failureCommand); - - if (failureResponse.events && failureResponse.events.length > 0) { - // Extract reason from the message - const { message } = failureResponse.events[0]; - const reasonMatch = message.match(/Reason:\s*([^]+?)(?:\s+at\s|$)/); - const failureReason = reasonMatch && reasonMatch[1] - ? reasonMatch[1].trim() - : message.trim(); - - return { executed: true, failureReason }; - } - - return { executed: true, failureReason: null }; - } catch (error) { - log.error(`Error getting audit status for ${auditType}:`, error); - return { executed: false, failureReason: null }; - } -} diff --git a/test/tasks/opportunity-status-processor/analyze-missing-opportunities.test.js b/test/tasks/opportunity-status-processor/analyze-missing-opportunities.test.js new file mode 100644 index 00000000..4055a2a9 --- /dev/null +++ b/test/tasks/opportunity-status-processor/analyze-missing-opportunities.test.js @@ -0,0 +1,101 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import { expect } from 'chai'; +import { analyzeMissingOpportunities } from '../../../src/tasks/opportunity-status-processor/handler.js'; + +describe('analyzeMissingOpportunities (DB-driven, no CloudWatch)', () => { + it('reports a missing opportunity as in-progress when its audit has not completed yet', () => { + const results = analyzeMissingOpportunities( + ['meta-tags'], + ['meta-tags'], + [], // no audits completed yet + { rum: true, seoImport: true, scraping: true }, + ); + + expect(results).to.deep.equal([{ + opportunity: 'meta-tags', + audit: 'meta-tags', + reason: 'meta-tags audit is still in progress', + inProgress: true, + }]); + }); + + it('reports unmet dependencies when the audit completed but a data source is missing', () => { + const results = analyzeMissingOpportunities( + ['meta-tags'], + ['meta-tags'], + ['meta-tags'], // completed + { rum: true, seoImport: true, scraping: false }, // scraping missing + ); + + expect(results).to.deep.equal([{ + opportunity: 'meta-tags', + audit: 'meta-tags', + reason: 'Missing dependencies: Scraping', + }]); + }); + + it('reports the SEO Import dependency by name when it is the missing source', () => { + const results = analyzeMissingOpportunities( + ['meta-tags'], + ['meta-tags'], + ['meta-tags'], // completed + { rum: true, seoImport: false, scraping: true }, // SEO import missing + ); + + expect(results).to.deep.equal([{ + opportunity: 'meta-tags', + audit: 'meta-tags', + reason: 'Missing dependencies: SEO Import', + }]); + }); + + it('reports "found no issues" when the audit completed with all dependencies met', () => { + const results = analyzeMissingOpportunities( + ['cwv'], + ['cwv'], + ['cwv'], // completed + { rum: true, seoImport: true, scraping: true }, + ); + + expect(results).to.deep.equal([{ + opportunity: 'cwv', + audit: 'cwv', + reason: 'Audit executed successfully, found no issues to report (no opportunities created)', + }]); + }); + + it('skips opportunities that have no related audit in the profile', () => { + const results = analyzeMissingOpportunities( + ['meta-tags'], + ['cwv'], // meta-tags not produced by any configured audit + ['cwv'], + { rum: true, seoImport: true, scraping: true }, + ); + + expect(results).to.deep.equal([]); + }); + + it('never claims an audit "has not been executed" (that was the CloudWatch false-negative)', () => { + const results = analyzeMissingOpportunities( + ['meta-tags'], + ['meta-tags'], + [], + { rum: true, seoImport: true, scraping: true }, + ); + + for (const r of results) { + expect(r.reason).to.not.match(/has not been executed/); + } + }); +}); diff --git a/test/tasks/opportunity-status-processor/delivery-type-applicability.test.js b/test/tasks/opportunity-status-processor/delivery-type-applicability.test.js new file mode 100644 index 00000000..6b38c400 --- /dev/null +++ b/test/tasks/opportunity-status-processor/delivery-type-applicability.test.js @@ -0,0 +1,35 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import { expect } from 'chai'; +import { isOpportunityApplicableForDeliveryType } from '../../../src/tasks/opportunity-status-processor/handler.js'; + +describe('isOpportunityApplicableForDeliveryType', () => { + it('excludes security-vulnerabilities for aem_edge sites (audit only runs on AEM_CS)', () => { + expect(isOpportunityApplicableForDeliveryType('security-vulnerabilities', 'aem_edge')) + .to.equal(false); + }); + + it('includes security-vulnerabilities for aem_cs sites', () => { + expect(isOpportunityApplicableForDeliveryType('security-vulnerabilities', 'aem_cs')) + .to.equal(true); + }); + + it('treats unrestricted opportunities as applicable for any delivery type', () => { + expect(isOpportunityApplicableForDeliveryType('meta-tags', 'aem_edge')).to.equal(true); + expect(isOpportunityApplicableForDeliveryType('cwv', 'other')).to.equal(true); + }); + + it('is applicable when delivery type is unknown/undefined (avoid hiding opportunities)', () => { + expect(isOpportunityApplicableForDeliveryType('meta-tags', undefined)).to.equal(true); + }); +}); diff --git a/test/tasks/opportunity-status-processor/derive-scraping-status.test.js b/test/tasks/opportunity-status-processor/derive-scraping-status.test.js new file mode 100644 index 00000000..0be456af --- /dev/null +++ b/test/tasks/opportunity-status-processor/derive-scraping-status.test.js @@ -0,0 +1,50 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import { expect } from 'chai'; +import { deriveScrapingStatus } from '../../../src/tasks/opportunity-status-processor/handler.js'; + +describe('deriveScrapingStatus', () => { + it('is "available" when at least one URL completed', () => { + expect(deriveScrapingStatus({ + completed: 5, failed: 2, pending: 10, total: 17, + })) + .to.equal('available'); + }); + + it('is "in_progress" when nothing completed but URLs are still pending/running', () => { + // The clover case: 0 completed, 159 failed, 829 still pending out of 988. + expect(deriveScrapingStatus({ + completed: 0, failed: 159, pending: 829, total: 988, + })) + .to.equal('in_progress'); + }); + + it('is "failed" only when terminal with zero completions', () => { + expect(deriveScrapingStatus({ + completed: 0, failed: 12, pending: 0, total: 12, + })) + .to.equal('failed'); + }); + + it('is "unknown" when there are no results yet', () => { + expect(deriveScrapingStatus({ + completed: 0, failed: 0, pending: 0, total: 0, + })) + .to.equal('unknown'); + }); + + it('is "unknown" when no stats are available', () => { + expect(deriveScrapingStatus(null)).to.equal('unknown'); + expect(deriveScrapingStatus(undefined)).to.equal('unknown'); + }); +}); diff --git a/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js b/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js index 8e627711..9a3dbe53 100644 --- a/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js +++ b/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js @@ -43,6 +43,7 @@ describe('Opportunity Status Processor', () => { mockSite = { getOpportunities: sandbox.stub().resolves([]), getSuggestions: sandbox.stub().resolves([]), + getDeliveryType: sandbox.stub().returns('aem_edge'), }; // Mock fetch for robots.txt and HEAD requests @@ -417,9 +418,6 @@ describe('Opportunity Status Processor', () => { '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, - '../../../src/utils/cloudwatch-utils.js': { - getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), - }, }); await Promise.all(testCases.map(async (testCase) => { @@ -439,6 +437,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), }), }, SiteTopPage: { @@ -477,6 +476,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), }), }, SiteTopPage: { @@ -507,9 +507,6 @@ describe('Opportunity Status Processor', () => { '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, - '../../../src/utils/cloudwatch-utils.js': { - getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), - }, }); await handler.runOpportunityStatusProcessor(testMessage, testContext); @@ -550,6 +547,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), }), }, SiteTopPage: { @@ -572,9 +570,6 @@ describe('Opportunity Status Processor', () => { '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, - '../../../src/utils/cloudwatch-utils.js': { - getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), - }, }); await handler.runOpportunityStatusProcessor(testMessage, testContext); @@ -640,6 +635,7 @@ describe('Opportunity Status Processor', () => { const testSiteMock = { getOpportunities: sinon.stub().resolves(testCase.opportunities), + getDeliveryType: sinon.stub().returns('aem_edge'), }; const testContext = { @@ -723,6 +719,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), }), }, SiteTopPage: { @@ -773,6 +770,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), }), }, SiteTopPage: { @@ -917,10 +915,6 @@ describe('Opportunity Status Processor', () => { '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, - '../../../src/utils/cloudwatch-utils.js': { - // Audit not executed (unmet dependencies) - getAuditStatus: sinon.stub().resolves({ executed: false, failureReason: null }), - }, }); // Set up audit types @@ -1361,192 +1355,6 @@ describe('Opportunity Status Processor', () => { }); }); - describe('CloudWatch Log Analysis - Deep Testing', () => { - let CloudWatchLogsClient; - let mockSendStub; - - beforeEach(async () => { - // Dynamically import CloudWatch Client - const CloudWatchModule = await import('@aws-sdk/client-cloudwatch-logs'); - CloudWatchLogsClient = CloudWatchModule.CloudWatchLogsClient; - - // Create a mock for CloudWatchLogsClient.prototype.send - mockSendStub = sinon.stub(CloudWatchLogsClient.prototype, 'send'); - - // Default: return empty events - mockSendStub.resolves({ events: [] }); - - context.mockCloudWatchSend = mockSendStub; - }); - - afterEach(() => { - if (mockSendStub && mockSendStub.restore) { - mockSendStub.restore(); - } - }); - - it('should detect audit execution in CloudWatch logs', async () => { - message.taskContext.auditTypes = ['cwv']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock CloudWatch to return audit execution event - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: `Received cwv audit request for: ${message.siteId}`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should extract failure reason with "Reason:" and "at" pattern', async () => { - message.taskContext.auditTypes = ['cwv']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock CloudWatch to return failure event - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: `cwv audit for ${message.siteId} failed. Reason: RUM data not available at line 123`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(mockSite.getOpportunities.called).to.be.true; - }); - - it('should analyze missing opportunities with all dependencies met', async () => { - message.taskContext.auditTypes = ['cwv']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock all services as available - context.dataAccess.SiteTopPage.allBySiteIdAndSourceAndGeo.resolves([ - { url: 'https://example.com/page1' }, - ]); - - // Mock audit was executed - context.mockCloudWatchSend.onFirstCall().resolves({ - events: [{ - timestamp: Date.now(), - message: `Received cwv audit request for: ${message.siteId}`, - }], - }); - - // Mock no failure found - should report as "unknown reason" - context.mockCloudWatchSend.onSecondCall().resolves({ events: [] }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should handle broken-internal-links with unmet RUM dependency', async () => { - message.taskContext.auditTypes = ['broken-internal-links']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // RUM not available, top-pages available - context.dataAccess.SiteTopPage.allBySiteIdAndSourceAndGeo.resolves([ - { url: 'https://example.com/page1' }, - ]); - - // Mock audit was executed - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: `Received broken-internal-links audit request for: ${message.siteId}`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should handle broken-internal-links with unmet top-pages dependency', async () => { - message.taskContext.auditTypes = ['broken-internal-links']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // No top pages - context.dataAccess.SiteTopPage.allBySiteIdAndSourceAndGeo.resolves([]); - - // Mock audit was executed - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: `Received broken-internal-links audit request for: ${message.siteId}`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should check scraping success rate with successful scrapes', async () => { - message.siteUrl = 'https://example.com'; - message.taskContext.onboardStartTime = Date.now() - 3600000; - - // Mock successful scrapes - context.mockCloudWatchSend.onCall(0).resolves({ - events: [ - { message: 'successfully scraped' }, - { message: 'successfully scraped' }, - { message: 'successfully scraped' }, - ], - }); - - // Mock failed scrapes - context.mockCloudWatchSend.onCall(1).resolves({ - events: [ - { message: 'failed to scrape' }, - ], - }); - - await runOpportunityStatusProcessor(message, context); - - // Should calculate success rate: 3/4 = 75% (above threshold) - expect(mockSite.getOpportunities.called).to.be.true; - }); - - it('should flag site as unavailable with low scraping success rate', async () => { - message.siteUrl = 'https://example.com'; - message.taskContext.onboardStartTime = Date.now() - 3600000; - - // Mock 2 successful scrapes - context.mockCloudWatchSend.onCall(0).resolves({ - events: [ - { message: 'successfully scraped' }, - { message: 'successfully scraped' }, - ], - }); - - // Mock 5 failed scrapes (2/7 = 28% success rate < 50% threshold) - context.mockCloudWatchSend.onCall(1).resolves({ - events: [ - { message: 'failed to scrape' }, - { message: 'failed to scrape' }, - { message: 'failed to scrape' }, - { message: 'failed to scrape' }, - { message: 'failed to scrape' }, - ], - }); - - await runOpportunityStatusProcessor(message, context); - - // Should flag as unavailable due to low success rate - expect(mockSite.getOpportunities.called).to.be.true; - }); - }); - describe('Complete Slack Output Flow', () => { it('should send all three sections with complete data', async () => { message.siteUrl = 'https://example.com'; @@ -1697,335 +1505,7 @@ describe('Opportunity Status Processor', () => { }); }); - describe('Missing Opportunity Analysis with Real Audits', () => { - let CloudWatchLogsClient; - let mockSendStub; - - beforeEach(async () => { - const CloudWatchModule = await import('@aws-sdk/client-cloudwatch-logs'); - CloudWatchLogsClient = CloudWatchModule.CloudWatchLogsClient; - mockSendStub = sinon.stub(CloudWatchLogsClient.prototype, 'send'); - mockSendStub.resolves({ events: [] }); - context.mockCloudWatchSend = mockSendStub; - }); - - afterEach(() => { - if (mockSendStub && mockSendStub.restore) { - mockSendStub.restore(); - } - }); - - it('should analyze meta-tags audit with missing top-pages', async () => { - message.taskContext.auditTypes = ['meta-tags']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // No top pages - context.dataAccess.SiteTopPage.allBySiteIdAndSourceAndGeo.resolves([]); - - // Mock audit was executed - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: `Received meta-tags audit request for: ${message.siteId}`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should analyze forms-opportunities audit', async () => { - message.taskContext.auditTypes = ['forms-opportunities']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock audit was executed - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: `Received forms-opportunities audit request for: ${message.siteId}`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should analyze experimentation-opportunities audit', async () => { - message.taskContext.auditTypes = ['experimentation-opportunities']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock audit was executed - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: `Received experimentation-opportunities audit request for: ${message.siteId}`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should analyze accessibility audit', async () => { - message.taskContext.auditTypes = ['accessibility']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock audit was executed - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: `Received accessibility audit request for: ${message.siteId}`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should analyze audit failure with detailed reason', async () => { - message.taskContext.auditTypes = ['cwv']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock audit was executed - context.mockCloudWatchSend.onFirstCall().resolves({ - events: [{ - timestamp: Date.now(), - message: `Received cwv audit request for: ${message.siteId}`, - }], - }); - - // Mock audit failure with reason - context.mockCloudWatchSend.onSecondCall().resolves({ - events: [{ - timestamp: Date.now(), - message: `cwv audit for ${message.siteId} failed. Reason: Timeout waiting for RUM data at runtime.js:123`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should handle scraping dependency', async () => { - message.siteUrl = 'https://example.com'; - message.taskContext.auditTypes = ['alt-text']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock scraping not available (site not reachable) - global.fetch.onFirstCall().resolves({ - ok: true, - status: 200, - text: sinon.stub().resolves('User-agent: *\nAllow: /'), - }); - global.fetch.onSecondCall().resolves({ - ok: false, - status: 500, - }); - - // Mock audit was executed - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: `Received alt-text audit request for: ${message.siteId}`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should trigger audit failure path (lines 616-620)', async () => { - // Use meta-tags which only depends on 'top-pages' (import) - message.taskContext.auditTypes = ['meta-tags']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); // meta-tags opportunity is missing - - // Mock import (top-pages) as available so dependency check passes - context.dataAccess.SiteTopPage.allBySiteIdAndSourceAndGeo - .withArgs(message.siteId) - .resolves([{ url: 'https://example.com/page1' }]); - - // Reset and configure CloudWatch calls - context.mockCloudWatchSend.reset(); - - // First call: getAuditStatus - audit WAS executed and has failure reason - context.mockCloudWatchSend.onCall(0).resolves({ - events: [{ - timestamp: Date.now(), - message: `Received meta-tags audit request for: ${message.siteId}`, - }], - }); - - // Second call: getAuditStatus - return a failure reason - context.mockCloudWatchSend.onCall(1).resolves({ - events: [{ - timestamp: Date.now(), - message: `meta-tags audit for ${message.siteId} failed. Reason: Unable to parse meta tags`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - // Verify the audit failure path was triggered - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should test getServicesNeedingLogAnalysis with all services available', async () => { - message.siteUrl = undefined; // No siteUrl to skip RUM/GSC/Scraping checks - message.taskContext.slackContext = { - channelId: 'test-channel', - threadTs: 'test-thread', - }; - - // Mock import and SEOImport as available - context.dataAccess.SiteTopPage.allBySiteIdAndSourceAndGeo = sinon.stub(); - context.dataAccess.SiteTopPage.allBySiteIdAndSourceAndGeo - .withArgs(message.siteId) - .resolves([{ url: 'https://example.com/page1' }]); - context.dataAccess.SiteTopPage.allBySiteIdAndSourceAndGeo - .withArgs(message.siteId, 'seo', 'global') - .resolves([{ url: 'https://example.com/page1', traffic: 1000 }]); - - const mockOpportunities = [ - { - getType: () => 'cwv', - getSuggestions: sinon.stub().resolves(['suggestion1']), - }, - ]; - mockSite.getOpportunities.resolves(mockOpportunities); - - await runOpportunityStatusProcessor(message, context); - - // When no siteUrl, RUM/GSC/Scraping are false, but SEOImport and Import are true - // This will trigger "Services requiring log analysis" log, - // not "All service preconditions passed" - // The test verifies the function executes without errors - expect(mockSite.getOpportunities.called).to.be.true; - }); - - it('should extract failure reason with Reason: pattern (lines 475-476)', async () => { - message.taskContext.auditTypes = ['cwv']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock audit execution - context.mockCloudWatchSend.onFirstCall().resolves({ - events: [{ - timestamp: Date.now(), - message: `Received cwv audit request for: ${message.siteId}`, - }], - }); - - // Mock failure with "Reason:" pattern (without "at") - context.mockCloudWatchSend.onSecondCall().resolves({ - events: [{ - timestamp: Date.now(), - message: `cwv audit for ${message.siteId} failed. Reason: Invalid RUM data format`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should handle CloudWatch error in getAuditStatus (lines 481-483)', async () => { - message.taskContext.auditTypes = ['cwv']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock audit execution - context.mockCloudWatchSend.onFirstCall().resolves({ - events: [{ - timestamp: Date.now(), - message: `Received cwv audit request for: ${message.siteId}`, - }], - }); - - // Mock CloudWatch error on second call - context.mockCloudWatchSend.onSecondCall().rejects(new Error('CloudWatch service error')); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - - it('should handle opportunity with no related audits (lines 557-560)', async () => { - message.taskContext.auditTypes = ['unknown-audit']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - await runOpportunityStatusProcessor(message, context); - - // Should complete without errors even with unknown audit type - expect(mockSite.getOpportunities.called).to.be.true; - }); - - it('should check scraping dependency for missing opportunity (lines 593-594)', async () => { - message.siteUrl = 'https://example.com'; - message.taskContext.auditTypes = ['alt-text']; - message.taskContext.onboardStartTime = Date.now() - 3600000; - mockSite.getOpportunities.resolves([]); - - // Mock robots.txt blocking scraping - global.fetch.resetBehavior(); - global.fetch.resolves({ - ok: true, - status: 200, - text: sinon.stub().resolves('User-agent: *\nDisallow: /'), - }); - - // Mock audit executed - context.mockCloudWatchSend.onFirstCall().resolves({ - events: [{ - timestamp: Date.now(), - message: `Received alt-text audit request for: ${message.siteId}`, - }], - }); - - await runOpportunityStatusProcessor(message, context); - - expect(context.log.warn.calledWithMatch('Missing opportunities')).to.be.true; - }); - }); - describe('GSC and Scraping Dependency Coverage', () => { - let CloudWatchLogsClient; - let mockSendStub; - - beforeEach(async () => { - // Dynamically import CloudWatch Client - const CloudWatchModule = await import('@aws-sdk/client-cloudwatch-logs'); - CloudWatchLogsClient = CloudWatchModule.CloudWatchLogsClient; - - // Create a mock for CloudWatchLogsClient.prototype.send - mockSendStub = sinon.stub(CloudWatchLogsClient.prototype, 'send'); - - // Default: return empty events - mockSendStub.resolves({ events: [] }); - - context.mockCloudWatchSend = mockSendStub; - }); - - afterEach(() => { - if (mockSendStub && mockSendStub.restore) { - mockSendStub.restore(); - } - delete context.mockCloudWatchSend; - }); - it('should cover scraping dependency when checked (lines 330-331, 454-457, 595-596, 628-638)', async () => { // Temporarily modify OPPORTUNITY_DEPENDENCY_MAP to include a scraping dependency const dependencyMapModule = await import('@adobe/spacecat-shared-utils'); @@ -2042,15 +1522,6 @@ describe('Opportunity Status Processor', () => { mockSite.getOpportunities.resolves([]); - // Reset CloudWatch to say audit was executed - context.mockCloudWatchSend.reset(); - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: 'Received broken-backlinks audit request for: test-site-id', - }], - }); - await runOpportunityStatusProcessor(message, context); // Should have tried to check scraping and detected it's not available @@ -2076,15 +1547,6 @@ describe('Opportunity Status Processor', () => { mockSite.getOpportunities.resolves([]); - // Reset CloudWatch to say audit was executed - context.mockCloudWatchSend.reset(); - context.mockCloudWatchSend.resolves({ - events: [{ - timestamp: Date.now(), - message: 'Received cwv audit request for: test-site-id', - }], - }); - await runOpportunityStatusProcessor(message, context); // Should have tried to check GSC @@ -2156,9 +1618,6 @@ describe('Opportunity Status Processor', () => { '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, - '../../../src/utils/cloudwatch-utils.js': { - getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), - }, }); dependencyMapModule.OPPORTUNITY_DEPENDENCY_MAP['broken-backlinks'] = ['scraping']; @@ -2476,6 +1935,50 @@ describe('Opportunity Status Processor', () => { }); }); + describe('Delivery-type-aware expectations (Issue B)', () => { + it('does not flag security-vulnerabilities as missing on an aem_edge site', async () => { + const mockSlackClient = { + postMessage: sinon.stub().resolves(), + }; + const SlackClientModule = await import('@adobe/spacecat-shared-slack-client'); + const slackStub = sinon.stub(SlackClientModule.BaseSlackClient, 'createFrom').returns(mockSlackClient); + + message.siteUrl = 'https://example.com'; + message.taskContext.auditTypes = ['security-vulnerabilities', 'meta-tags']; + const onboardStartTime = Date.now() - 3600000; + message.taskContext.onboardStartTime = onboardStartTime; + message.taskContext.slackContext = { channelId: 'test-channel', threadTs: 'test-thread' }; + context.env.AWS_REGION = 'us-east-1'; + + // aem_edge site: security-vulnerabilities audit can never run here. + mockSite.getDeliveryType.returns('aem_edge'); + + // Both audits have completed (fresh records), and no opportunities were produced. + context.dataAccess.Audit = { + allLatestForSite: sinon.stub().resolves([ + { getAuditType: () => 'security-vulnerabilities', getAuditedAt: () => new Date(onboardStartTime + 1000).toISOString() }, + { getAuditType: () => 'meta-tags', getAuditedAt: () => new Date(onboardStartTime + 1000).toISOString() }, + ]), + }; + mockSite.getOpportunities.resolves([]); + + try { + await runOpportunityStatusProcessor(message, context); + + const allMessages = mockSlackClient.postMessage.getCalls() + .map((c) => c.args[0]?.text) + .join('\n'); + + // security-vulnerabilities is filtered out for aem_edge — never reported as missing. + expect(allMessages).to.not.contain('security-vulnerabilities'); + // A non-restricted audit is still evaluated and surfaced. + expect(allMessages).to.contain('meta-tags'); + } finally { + slackStub.restore(); + } + }); + }); + describe('Additional coverage for uncovered lines', () => { it('should use info icon for opportunities with no suggestions (line 621)', async () => { // Mock Slack client @@ -3853,9 +3356,6 @@ describe('Opportunity Status Processor', () => { '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, - '../../../src/utils/cloudwatch-utils.js': { - getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), - }, }); // Temporarily add scraping dependency @@ -3882,6 +3382,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), }), }, SiteTopPage: { @@ -3908,9 +3409,6 @@ describe('Opportunity Status Processor', () => { '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, - '../../../src/utils/cloudwatch-utils.js': { - getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), - }, }); const testMessage = { @@ -3930,6 +3428,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), }), }, SiteTopPage: { @@ -3952,9 +3451,6 @@ describe('Opportunity Status Processor', () => { '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, - '../../../src/utils/cloudwatch-utils.js': { - getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), - }, }); // Mock the audit-opportunity-map to create a scenario where @@ -3979,6 +3475,7 @@ describe('Opportunity Status Processor', () => { // But the filter in analyzeMissingOpportunities will check if 'alt-text' is in // the opportunities that can be generated by 'alt-text' audit getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), }), }, SiteTopPage: { @@ -4013,9 +3510,6 @@ describe('Opportunity Status Processor', () => { '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, - '../../../src/utils/cloudwatch-utils.js': { - getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), - }, }); // Create a scenario where an opportunity type exists but no audits in auditTypes @@ -4045,6 +3539,7 @@ describe('Opportunity Status Processor', () => { findById: sinon.stub().resolves({ // Site has cwv opportunity getOpportunities: sinon.stub().resolves([mockOpportunity]), + getDeliveryType: sinon.stub().returns('aem_edge'), getBaseURL: sinon.stub().returns('https://example.com'), }), }, @@ -4106,6 +3601,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), getBaseURL: sinon.stub().returns('https://example.com'), }), }, @@ -4151,9 +3647,6 @@ describe('Opportunity Status Processor', () => { createFrom: sinon.stub().returns({ getScrapeJobsByBaseURL: sinon.stub().resolves([]) }), }, }, - '../../../src/utils/cloudwatch-utils.js': { - getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), - }, '../../../src/utils/bot-detection.js': { checkAndAlertBotProtection: sinon.stub().resolves(null), }, @@ -4181,6 +3674,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), getBaseURL: sinon.stub().returns('https://example.com'), }), }, @@ -4294,6 +3788,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([cwvOpp]), + getDeliveryType: sinon.stub().returns('aem_edge'), getBaseURL: sinon.stub().returns('https://example.com'), }), }, @@ -4327,6 +3822,7 @@ describe('Opportunity Status Processor', () => { Site: { findById: sinon.stub().resolves({ getOpportunities: sinon.stub().resolves([]), + getDeliveryType: sinon.stub().returns('aem_edge'), getBaseURL: sinon.stub().returns('https://example.com'), }), }, diff --git a/test/utils/cloudwatch-utils.test.js b/test/utils/cloudwatch-utils.test.js deleted file mode 100644 index ddafd7ed..00000000 --- a/test/utils/cloudwatch-utils.test.js +++ /dev/null @@ -1,268 +0,0 @@ -/* - * Copyright 2025 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -import { expect } from 'chai'; -import sinon from 'sinon'; -import { CloudWatchLogsClient, FilterLogEventsCommand } from '@aws-sdk/client-cloudwatch-logs'; -import { getAuditStatus } from '../../src/utils/cloudwatch-utils.js'; - -describe('CloudWatch Utils', () => { - let mockContext; - let sandbox; - let mockSendStub; - - beforeEach(() => { - sandbox = sinon.createSandbox(); - mockContext = { - env: { - AWS_REGION: 'us-east-1', - AUDIT_WORKER_LOG_GROUP: '/aws/lambda/spacecat-services--audit-worker', - }, - log: { - info: sandbox.stub(), - debug: sandbox.stub(), - warn: sandbox.stub(), - error: sandbox.stub(), - }, - }; - - // Stub CloudWatchLogsClient.prototype.send - mockSendStub = sandbox.stub(CloudWatchLogsClient.prototype, 'send'); - }); - - afterEach(() => { - sandbox.restore(); - }); - - describe('getAuditStatus', () => { - const auditType = 'cwv'; - const siteId = 'test-site-id'; - const onboardStartTime = Date.now() - 3600000; // 1 hour ago - - it('should return executed: false when no execution log found', async () => { - mockSendStub.resolves({ - events: [], - }); - - const result = await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - expect(result).to.deep.equal({ executed: false, failureReason: null }); - expect(mockSendStub).to.have.been.calledOnce; - expect(mockContext.log.error).to.not.have.been.called; - }); - - it('should return executed: true, failureReason: null when audit executed successfully', async () => { - // First call: execution found - mockSendStub.onFirstCall().resolves({ - events: [ - { message: `Received ${auditType} audit request for: ${siteId}` }, - ], - }); - - // Second call: no failure found - mockSendStub.onSecondCall().resolves({ - events: [], - }); - - const result = await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - expect(result).to.deep.equal({ executed: true, failureReason: null }); - expect(mockSendStub).to.have.been.calledTwice; - }); - - it('should return executed: true with failureReason when audit failed', async () => { - const failureMessage = `${auditType} audit for ${siteId} failed. Reason: Connection timeout`; - - // First call: execution found - mockSendStub.onFirstCall().resolves({ - events: [ - { message: `Received ${auditType} audit request for: ${siteId}` }, - ], - }); - - // Second call: failure found - mockSendStub.onSecondCall().resolves({ - events: [ - { message: failureMessage }, - ], - }); - - const result = await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - expect(result).to.deep.equal({ - executed: true, - failureReason: 'Connection timeout', - }); - expect(mockSendStub).to.have.been.calledTwice; - }); - - it('should extract failure reason from message with stack trace', async () => { - const failureMessage = `${auditType} audit for ${siteId} failed. Reason: Database error at Error: ...`; - - mockSendStub.onFirstCall().resolves({ - events: [ - { message: `Received ${auditType} audit request for: ${siteId}` }, - ], - }); - - mockSendStub.onSecondCall().resolves({ - events: [ - { message: failureMessage }, - ], - }); - - const result = await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - expect(result.failureReason).to.equal('Database error'); - }); - - it('should return full message as failureReason when no reason pattern matches', async () => { - const failureMessage = `${auditType} audit for ${siteId} failed with unknown error format`; - - mockSendStub.onFirstCall().resolves({ - events: [ - { message: `Received ${auditType} audit request for: ${siteId}` }, - ], - }); - - mockSendStub.onSecondCall().resolves({ - events: [ - { message: failureMessage }, - ], - }); - - const result = await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - expect(result.failureReason).to.equal(failureMessage); - }); - - it('should use custom log group from env if provided', async () => { - mockContext.env.AUDIT_WORKER_LOG_GROUP = '/custom/log/group'; - mockSendStub.resolves({ events: [] }); - - await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - const { firstCall } = mockSendStub; - expect(firstCall.args[0]).to.be.instanceOf(FilterLogEventsCommand); - expect(firstCall.args[0].input.logGroupName).to.equal('/custom/log/group'); - }); - - it('should use default log group when env variable not set', async () => { - delete mockContext.env.AUDIT_WORKER_LOG_GROUP; - mockSendStub.resolves({ events: [] }); - - await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - const { firstCall } = mockSendStub; - expect(firstCall.args[0].input.logGroupName).to.equal('/aws/lambda/spacecat-services--audit-worker'); - }); - - it('should use default AWS_REGION when not provided', async () => { - delete mockContext.env.AWS_REGION; - mockSendStub.resolves({ events: [] }); - - await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - // CloudWatchLogsClient should be created with default region - expect(mockSendStub).to.have.been.called; - }); - - it('should calculate search window with 5 minute buffer for execution check', async () => { - const now = Date.now(); - const onboardTime = now - 3600000; // 1 hour ago - mockSendStub.resolves({ events: [] }); - - await getAuditStatus(auditType, siteId, onboardTime, mockContext); - - const { firstCall } = mockSendStub; - const command = firstCall.args[0]; - const expectedStartTime = onboardTime - (5 * 60 * 1000); // 5 min buffer - - // Allow 1 second tolerance for timing - expect(command.input.startTime).to.be.closeTo(expectedStartTime, 1000); - expect(command.input.endTime).to.be.closeTo(now, 1000); - }); - - it('should calculate search window with 30 second buffer for failure check', async () => { - const now = Date.now(); - const onboardTime = now - 3600000; // 1 hour ago - - mockSendStub.onFirstCall().resolves({ - events: [ - { message: `Received ${auditType} audit request for: ${siteId}` }, - ], - }); - mockSendStub.onSecondCall().resolves({ events: [] }); - - await getAuditStatus(auditType, siteId, onboardTime, mockContext); - - const { secondCall } = mockSendStub; - const command = secondCall.args[0]; - const expectedStartTime = onboardTime - (30 * 1000); // 30 sec buffer - - // Allow 1 second tolerance for timing - expect(command.input.startTime).to.be.closeTo(expectedStartTime, 1000); - }); - - it('should use 30 minute fallback when onboardStartTime is not provided', async () => { - const now = Date.now(); - mockSendStub.resolves({ events: [] }); - - await getAuditStatus(auditType, siteId, null, mockContext); - - const { firstCall } = mockSendStub; - const command = firstCall.args[0]; - const expectedStartTime = now - (30 * 60 * 1000); // 30 min fallback - - // Allow 1 second tolerance for timing - expect(command.input.startTime).to.be.closeTo(expectedStartTime, 1000); - }); - - it('should handle CloudWatch API errors gracefully', async () => { - const error = new Error('CloudWatch API error'); - mockSendStub.rejects(error); - - const result = await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - expect(result).to.deep.equal({ executed: false, failureReason: null }); - expect(mockContext.log.error).to.have.been.calledWithMatch( - /Error getting audit status for cwv/, - ); - expect(mockContext.log.error.firstCall.args[1]).to.equal(error); - }); - - it('should use correct filter pattern for execution check', async () => { - mockSendStub.resolves({ events: [] }); - - await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - const { firstCall } = mockSendStub; - const command = firstCall.args[0]; - expect(command.input.filterPattern).to.equal(`"Received ${auditType} audit request for: ${siteId}"`); - }); - - it('should use correct filter pattern for failure check', async () => { - mockSendStub.onFirstCall().resolves({ - events: [ - { message: `Received ${auditType} audit request for: ${siteId}` }, - ], - }); - mockSendStub.onSecondCall().resolves({ events: [] }); - - await getAuditStatus(auditType, siteId, onboardStartTime, mockContext); - - const { secondCall } = mockSendStub; - const command = secondCall.args[0]; - expect(command.input.filterPattern).to.equal(`"${auditType} audit for ${siteId} failed"`); - }); - }); -}); From 40e830ceb8ae7adef567d2b24aebbd7b87bae1a3 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Thu, 30 Jul 2026 17:38:29 -0500 Subject: [PATCH 2/3] test(onboard-status): cover info-icon rendering for completed no-issue audits Adds a handler-level test for a completed audit that produced no opportunity (rendered with :information_source: in the Audit Processing Errors section), closing the codecov/patch gap on handler.js. Co-Authored-By: Claude Opus 4.8 --- .../opportunity-status-processor.test.js | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js b/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js index 9a3dbe53..60514361 100644 --- a/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js +++ b/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js @@ -1979,6 +1979,52 @@ describe('Opportunity Status Processor', () => { }); }); + describe('Audit Processing Errors rendering', () => { + it('marks a completed audit that produced no opportunity with an info icon', async () => { + const mockSlackClient = { + postMessage: sinon.stub().resolves(), + }; + const SlackClientModule = await import('@adobe/spacecat-shared-slack-client'); + const slackStub = sinon.stub(SlackClientModule.BaseSlackClient, 'createFrom').returns(mockSlackClient); + + // Make RUM available so cwv's only trackable dependency is met. + const RUMAPIClientModule = await import('@adobe/spacecat-shared-rum-api-client'); + const originalRumCreateFrom = RUMAPIClientModule.default.createFrom; + RUMAPIClientModule.default.createFrom = sinon.stub().returns({ + retrieveDomainkey: sinon.stub().resolves('test-key'), + }); + + message.siteUrl = 'https://example.com'; + message.taskContext.auditTypes = ['cwv']; + const onboardStartTime = Date.now() - 3600000; + message.taskContext.onboardStartTime = onboardStartTime; + message.taskContext.slackContext = { channelId: 'test-channel', threadTs: 'test-thread' }; + context.env.AWS_REGION = 'us-east-1'; + + // cwv completed (fresh record) but produced no opportunity. + context.dataAccess.Audit = { + allLatestForSite: sinon.stub().resolves([ + { getAuditType: () => 'cwv', getAuditedAt: () => new Date(onboardStartTime + 1000).toISOString() }, + ]), + }; + mockSite.getOpportunities.resolves([]); + + try { + await runOpportunityStatusProcessor(message, context); + + const allMessages = mockSlackClient.postMessage.getCalls() + .map((c) => c.args[0]?.text) + .join('\n'); + + expect(allMessages).to.contain('found no issues to report'); + expect(allMessages).to.contain(':information_source:'); + } finally { + slackStub.restore(); + RUMAPIClientModule.default.createFrom = originalRumCreateFrom; + } + }); + }); + describe('Additional coverage for uncovered lines', () => { it('should use info icon for opportunities with no suggestions (line 621)', async () => { // Mock Slack client From e77ebaf965e1fbc65eda2333edb48520cda96524 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Mon, 3 Aug 2026 11:59:27 -0500 Subject: [PATCH 3/3] docs(onboard-status): cite scrape_url_status enum for the pending predicate Addresses review nit: document that PENDING/RUNNING are the only non-terminal scrape_url_status values (enum: PENDING, RUNNING, REDIRECT, COMPLETE, FAILED, STOPPED), so the in-progress predicate is provably complete and REDIRECT/FAILED/ STOPPED correctly read as failed when nothing has completed. Co-Authored-By: Claude Opus 4.8 --- src/tasks/opportunity-status-processor/handler.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tasks/opportunity-status-processor/handler.js b/src/tasks/opportunity-status-processor/handler.js index 81f5eb31..fcc1b851 100644 --- a/src/tasks/opportunity-status-processor/handler.js +++ b/src/tasks/opportunity-status-processor/handler.js @@ -239,6 +239,10 @@ async function isScrapingAvailable(baseUrl, context, onboardStartTime) { const completedCount = allUrlResults.filter((result) => result.status === 'COMPLETE').length; const failedCount = allUrlResults.filter((result) => result.status === 'FAILED').length; // Non-terminal URLs (scrape still running) — used to distinguish in-progress from failed. + // The scrape_url_status enum is exactly { PENDING, RUNNING, REDIRECT, COMPLETE, FAILED, + // STOPPED } (@mysticat/data-service-types); PENDING/RUNNING are the only non-terminal + // states, so this predicate is complete. REDIRECT/FAILED/STOPPED are terminal + // non-successes and correctly fall through to "failed" when nothing has completed. const pendingCount = allUrlResults.filter( (result) => result.status === 'PENDING' || result.status === 'RUNNING', ).length;