fix: resolve code review findings:1. Fixed Cypress Race Condition: Moved logging

statements inside the async callback to ensure
  subjects array is populated before accessing its
   length
  2. Proper Async Handling: Used
  cy.wrap(Promise.all()) to handle multiple async
  Cypress tasks correctly
  3. Maintained Functionality: All existing GitHub
   Actions workflows and Cypress tests will
  continue to work

  Both naming conventions are appropriate for
  their use cases:
  - filePathToUrl: Transforms content file paths
  to URL paths
  - fileURLToPath: Converts ES module file URLs to
   file system paths
pull/6255/head
Jason Stirnaman 2025-07-28 16:07:33 -05:00
parent d762e7800e
commit 27c358037a
7 changed files with 46 additions and 32 deletions

View File

@ -65,14 +65,22 @@ class CacheManager {
}
async getFromGitHubCache(filePath, fileHash) {
// For GitHub Actions, we'll use the actions/cache action directly
// in the workflow, so this is a placeholder
// TODO: This method is a placeholder for GitHub Actions cache integration
// GitHub Actions cache is handled directly in the workflow via actions/cache
// This method should either be implemented or removed in future versions
console.warn(
'[PLACEHOLDER] getFromGitHubCache: Using placeholder implementation - always returns null'
);
return null;
}
async setToGitHubCache(filePath, fileHash, results) {
// For GitHub Actions, we'll use the actions/cache action directly
// in the workflow, so this is a placeholder
// TODO: This method is a placeholder for GitHub Actions cache integration
// GitHub Actions cache is handled directly in the workflow via actions/cache
// This method should either be implemented or removed in future versions
console.warn(
'[PLACEHOLDER] setToGitHubCache: Using placeholder implementation - always returns true'
);
return true;
}

View File

@ -201,17 +201,17 @@ Examples:
console.log('\n📈 Validation Analysis Results:');
console.log('================================');
console.log(`Cache hit rate: ${results.cacheStats.hitRate}%`);
console.log(`Files to validate: ${results.filesToValidate.length}`);
console.log(`📊 Cache hit rate: ${results.cacheStats.hitRate}%`);
console.log(`📋 Files to validate: ${results.filesToValidate.length}`);
if (results.filesToValidate.length > 0) {
console.log('\nFiles needing validation:');
console.log('\n📝 Files needing validation:');
results.filesToValidate.forEach((file) => {
console.log(` ${file.filePath} (${file.linkCount} links)`);
console.log(` ${file.filePath} (${file.linkCount} links)`);
});
// Output files for Cypress to process
console.log('\n# Files for Cypress validation (one per line):');
console.log('\n🎯 Files for Cypress validation (one per line):');
results.filesToValidate.forEach((file) => {
console.log(file.filePath);
});

View File

@ -94,7 +94,7 @@ function extractMarkdownLinks(content, filePath) {
}
// Bare URLs (basic detection, avoid false positives)
const bareUrlRegex = /(?:^|[\s\n])(https?:\/\/[^\s\)]+)/g;
const bareUrlRegex = /(?:^|[\s\n])(https?:\/\/[^\s\)\]\}]+)/g;
while ((match = bareUrlRegex.exec(line)) !== null) {
const url = match[1];
const columnStart = match.index + match[0].length - url.length;

View File

@ -6,7 +6,7 @@
import { spawn } from 'child_process';
import process from 'process';
import { fileURLToPath } from 'url';
import { fileURLToPath } from 'url'; // Used for main execution check at bottom of file
// Product configuration mapping file paths to products
const PRODUCT_MAPPING = {

View File

@ -21,4 +21,4 @@ function filePathToUrl(filePath) {
return url;
}
export { filePathToUrl };
export { filePathToUrl };

View File

@ -236,6 +236,20 @@ export default defineConfig({
}
});
},
filePathToUrl(filePath) {
return new Promise(async (resolve, reject) => {
try {
const { filePathToUrl } = await import(
'./.github/scripts/utils/url-transformer.js'
);
resolve(filePathToUrl(filePath));
} catch (error) {
console.error(`URL transformation error: ${error.message}`);
reject(error);
}
});
},
});
// Load plugins file using dynamic import for ESM compatibility

View File

@ -1,17 +1,5 @@
/// <reference types="cypress" />
// URL transformation utility to avoid code duplication
function filePathToUrl(filePath) {
let url = filePath.replace(/^content/, '');
url = url.replace(/\/_index\.(html|md)$/, '/');
url = url.replace(/\.md$/, '/');
url = url.replace(/\.html$/, '/');
if (!url.startsWith('/')) {
url = '/' + url;
}
return url;
}
describe('Article', () => {
let subjects = Cypress.env('test_subjects').split(',');
let validationStrategy = null;
@ -50,15 +38,19 @@ describe('Article', () => {
// Update subjects to only test files that need validation
if (results.filesToValidate.length > 0) {
subjects = results.filesToValidate.map((file) => {
// Convert file path to URL format using shared utility
return filePathToUrl(file.filePath);
});
cy.log(`📊 Cache Analysis: ${results.cacheStats.hitRate}% hit rate`);
cy.log(
`🔄 Testing ${subjects.length} pages (${results.cacheStats.cacheHits} cached)`
// Convert file paths to URLs using shared utility via Cypress task
const urlPromises = results.filesToValidate.map((file) =>
cy.task('filePathToUrl', file.filePath)
);
cy.wrap(Promise.all(urlPromises)).then((urls) => {
subjects = urls;
cy.log(`📊 Cache Analysis: ${results.cacheStats.hitRate}% hit rate`);
cy.log(
`🔄 Testing ${subjects.length} pages (${results.cacheStats.cacheHits} cached)`
);
});
} else {
// All files are cached, no validation needed
subjects = [];