PR feedback

- update console.logs to core.infos

Author: abdulahmad307

.github/actions/find/README.md

@@ -25,7 +25,7 @@ https://primer.style/octicons/
 
 #### `scans`
 
-**Optional** Stringified JSON array of scans (string) to perform. If not provided, only axe will be performed. Valid options currently include 'axe'
+**Optional** Stringified JSON array of scans (string) to perform. If not provided, only axe will be performed.
 
 ### Outputs
 

.github/actions/find/src/findForUrl.ts

@@ -3,9 +3,10 @@ import AxeBuilder from '@axe-core/playwright'
 import playwright from 'playwright'
 import {AuthContext} from './AuthContext.js'
 import {generateScreenshots} from './generateScreenshots.js'
-import {loadPlugins} from './pluginManager.js'
+import {loadPlugins, invokePlugin} from './pluginManager.js'
 import {getScansContext} from './scansContextProvider.js'
 import axe from 'axe-core'
+import core from '@actions/core'
 
 export async function findForUrl(
   url: string,
@@ -20,7 +21,7 @@ export async function findForUrl(
   const context = await browser.newContext(contextOptions)
   const page = await context.newPage()
   await page.goto(url)
-  console.log(`Scanning ${page.url()}`)
+  core.info(`Scanning ${page.url()}`)
 
   const findings: Finding[] = []
   const addFinding = (findingData: Finding) => {
@@ -35,22 +36,14 @@ export async function findForUrl(
       rawFindings = await new AxeBuilder({page}).analyze()
     }
 
-    // - this if condition is not required, but makes it easier to make assertions
-    //   in unit tests on whether 'loadPlugins' was called or not (it does have the added
-    //   benefit of completely skipping plugin loading if we just want axe - so thats
-    //   a minor performance boost)
-    // - alternatively, we can wrap the 'plugin.default(...)' call in another function
-    //   and make assertions on whether that function was called or not
-    // - the other option is to wrap each plugin in a class instance
-    //   and make assertions on something like 'plugin.run' being called or not
     if (scansContext.shouldRunPlugins) {
       const plugins = await loadPlugins()
       for (const plugin of plugins) {
         if (scansContext.scansToPerform.includes(plugin.name)) {
-          console.log('Running plugin: ', plugin.name)
-          await plugin.default({page, addFinding, url})
+          core.info(`Running plugin: ${plugin.name}`)
+          await invokePlugin({plugin, page, addFinding, url})
         } else {
-          console.log(`Skipping plugin ${plugin.name} because it is not included in the 'scans' input`)
+          core.info(`Skipping plugin ${plugin.name} because it is not included in the 'scans' input`)
         }
       }
     }
@@ -73,7 +66,7 @@ export async function findForUrl(
     }))
     findings.push(...(axeFindings || []))
   } catch (e) {
-    console.error('Error during accessibility scan:', e)
+    core.error(`Error during accessibility scan: ${e}`)
   }
   await context.close()
   await browser.close()

.github/actions/find/src/generateScreenshots.ts

@@ -2,6 +2,7 @@ import fs from 'node:fs'
 import path from 'node:path'
 import crypto from 'node:crypto'
 import type {Page} from 'playwright'
+import core from '@actions/core'
 
 // Use GITHUB_WORKSPACE to ensure screenshots are saved in the workflow workspace root
 // where the artifact upload step can find them
@@ -12,9 +13,9 @@ export const generateScreenshots = async function (page: Page) {
   // Ensure screenshot directory exists
   if (!fs.existsSync(SCREENSHOT_DIR)) {
     fs.mkdirSync(SCREENSHOT_DIR, {recursive: true})
-    console.log(`Created screenshot directory: ${SCREENSHOT_DIR}`)
+    core.info(`Created screenshot directory: ${SCREENSHOT_DIR}`)
   } else {
-    console.log(`Using existing screenshot directory ${SCREENSHOT_DIR}`)
+    core.info(`Using existing screenshot directory ${SCREENSHOT_DIR}`)
   }
 
   try {
@@ -28,9 +29,9 @@ export const generateScreenshots = async function (page: Page) {
     const filepath = path.join(SCREENSHOT_DIR, filename)
 
     fs.writeFileSync(filepath, screenshotBuffer)
-    console.log(`Screenshot saved: ${filename}`)
+    core.info(`Screenshot saved: ${filename}`)
   } catch (error) {
-    console.error('Failed to capture/save screenshot:', error)
+    core.error(`Failed to capture/save screenshot: ${error}`)
     screenshotId = undefined
   }
 

.github/actions/find/src/pluginManager.ts

@@ -4,6 +4,7 @@ import {fileURLToPath} from 'url'
 import {dynamicImport} from './dynamicImport.js'
 import type {Finding} from './types.d.js'
 import playwright from 'playwright'
+import core from '@actions/core'
 
 // Helper to get __dirname equivalent in ES Modules
 const __filename = fileURLToPath(import.meta.url)
@@ -18,7 +19,7 @@ const plugins: Plugin[] = []
 let pluginsLoaded = false
 
 export async function loadPlugins() {
-  console.log('loading plugins')
+  core.info('loading plugins')
 
   try {
     if (!pluginsLoaded) {
@@ -27,7 +28,7 @@ export async function loadPlugins() {
     }
   } catch {
     plugins.length = 0
-    console.log(abortError)
+    core.error(abortError)
   } finally {
     pluginsLoaded = true
     return plugins
@@ -47,7 +48,7 @@ export function clearCache() {
 
 // exported for mocking/testing. not for actual use
 export async function loadBuiltInPlugins() {
-  console.log('Loading built-in plugins')
+  core.info('Loading built-in plugins')
 
   const pluginsPath = '../../../scanner-plugins/'
   await loadPluginsFromPath({
@@ -58,7 +59,7 @@ export async function loadBuiltInPlugins() {
 
 // exported for mocking/testing. not for actual use
 export async function loadCustomPlugins() {
-  console.log('Loading custom plugins')
+  core.info('Loading custom plugins')
 
   const pluginsPath = process.cwd() + '/.github/scanner-plugins/'
   await loadPluginsFromPath({
@@ -74,15 +75,26 @@ export async function loadPluginsFromPath({readPath, importPath}: {readPath: str
     for (const pluginFolder of res) {
       const pluginFolderPath = path.join(importPath, pluginFolder)
       if (fs.lstatSync(pluginFolderPath).isDirectory()) {
-        console.log('Found plugin: ', pluginFolder)
+        core.info(`Found plugin: ${pluginFolder}`)
         plugins.push(await dynamicImport(path.join(importPath, pluginFolder, '/index.js')))
       }
     }
   } catch (e) {
     // - log errors here for granular info
-    console.log('error: ')
-    console.log(e)
+    core.error('error: ')
+    core.error(e as Error)
     // - throw error to handle aborting the plugin scans
     throw e
   }
 }
+
+
+type InvokePluginParams = {
+  plugin: Plugin,
+  page: playwright.Page,
+  addFinding: (findingData: Finding) => void,
+  url: string
+}
+export function invokePlugin({ plugin, page, addFinding, url }: InvokePluginParams) {
+  return plugin.default({page, addFinding, url})
+}

.github/actions/find/src/scansContextProvider.ts

@@ -9,8 +9,8 @@ let scansContext: ScansContext | undefined
 
 export function getScansContext() {
   if (!scansContext) {
-    const scansJson = core.getInput('scans', {required: false})
-    const scansToPerform = JSON.parse(scansJson || '[]')
+    const scansInput = core.getInput('scans', {required: false})
+    const scansToPerform = JSON.parse(scansInput || '[]')
     // - if we don't have a scans input
     //   or we do have a scans input, but it only has 1 item and its 'axe'
     //   then we only want to run 'axe' and not the plugins
@@ -23,7 +23,7 @@ export function getScansContext() {
       //   (only axe scan) for backwards compatability.
       // - we can enforce using the 'scans' input in a future major release and
       //   mark it as required
-      shouldPerformAxeScan: !scansJson || scansToPerform.includes('axe'),
+      shouldPerformAxeScan: !scansInput || scansToPerform.includes('axe'),
       shouldRunPlugins: scansToPerform.length > 0 && !onlyAxeScan,
     }
   }

.github/actions/find/tests/findForUrl.test.ts

@@ -42,13 +42,15 @@ function clearAll() {
 describe('findForUrl', () => {
   vi.spyOn(core, 'getInput').mockImplementation(() => actionInput)
   vi.spyOn(pluginManager, 'loadPlugins').mockImplementation(() => Promise.resolve(loadedPlugins))
+  vi.spyOn(pluginManager, 'invokePlugin')
 
   async function axeOnlyTest() {
     clearAll()
 
     await findForUrl('test.com')
     expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1)
     expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(0)
+    expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(0)
   }
 
   describe('when no scans list is provided', () => {
@@ -68,23 +70,35 @@ describe('findForUrl', () => {
 
     describe('and the list includes axe and other scans', () => {
       it('runs axe and plugins', async () => {
-        actionInput = JSON.stringify(['axe', 'custom-scan'])
+        loadedPlugins = [
+          {name: 'custom-scan-1', default: vi.fn()},
+          {name: 'custom-scan-2', default: vi.fn()},
+        ]
+
+        actionInput = JSON.stringify(['axe', 'custom-scan-1'])
         clearAll()
 
         await findForUrl('test.com')
         expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1)
         expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1)
+        expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(1)
       })
     })
 
     describe('and the list does not include axe', () => {
       it('only runs plugins', async () => {
-        actionInput = JSON.stringify(['custom-scan'])
+        loadedPlugins = [
+          {name: 'custom-scan-1', default: vi.fn()},
+          {name: 'custom-scan-2', default: vi.fn()},
+        ]
+
+        actionInput = JSON.stringify(['custom-scan-1', 'custom-scan-2'])
         clearAll()
 
         await findForUrl('test.com')
         expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(0)
         expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1)
+        expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(2)
       })
     })
 

.github/actions/find/tests/pluginManager.test.ts

@@ -3,6 +3,7 @@ import {describe, it, expect, vi, beforeEach} from 'vitest'
 import * as fs from 'fs'
 import * as dynamicImportModule from '../src/dynamicImport.js'
 import * as pluginManager from '../src/pluginManager.js'
+import core from '@actions/core'
 
 // - enable spying on fs
 // https://vitest.dev/guide/browser/#limitations
@@ -52,10 +53,10 @@ describe('loadPlugins', () => {
 
     it('Aborts loading all plugins', async () => {
       pluginManager.clearCache()
-      const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
+      const logSpy = vi.spyOn(core, 'error').mockImplementation(() => {})
       const plugins = await pluginManager.loadPlugins()
       expect(plugins.length).toBe(0)
-      expect(consoleLogSpy).toHaveBeenCalledWith(pluginManager.abortError)
+      expect(logSpy).toHaveBeenCalledWith(pluginManager.abortError)
     })
   })
 })