From 18e5d1298e2774c7a0d4ff326b65b329dcfa1568 Mon Sep 17 00:00:00 2001 From: "Thomas.G" Date: Sun, 7 Jul 2024 23:26:52 +0200 Subject: [PATCH] refactor: improve utils by splitting them & adding UT (#373) --- README.md | 10 ++-- package.json | 12 ++--- src/analysis/fetch.js | 43 +++++++++-------- src/reporting/html.js | 7 ++- src/reporting/index.js | 2 +- src/reporting/pdf.js | 12 +++-- src/utils.js | 69 --------------------------- src/utils/cleanReportName.js | 26 ++++++++++ src/utils/cloneGITRepository.js | 32 +++++++++++++ src/utils/formatNpmPackages.js | 21 ++++++++ src/utils/index.js | 4 ++ src/utils/runInSpinner.js | 27 +++++++++++ test/test.js | 3 -- test/utils/cleanReportName.spec.js | 26 ++++++++++ test/utils/cloneGITRepository.spec.js | 34 +++++++++++++ test/utils/formatNpmPackages.spec.js | 41 ++++++++++++++++ 16 files changed, 260 insertions(+), 109 deletions(-) delete mode 100644 src/utils.js create mode 100644 src/utils/cleanReportName.js create mode 100644 src/utils/cloneGITRepository.js create mode 100644 src/utils/formatNpmPackages.js create mode 100644 src/utils/index.js create mode 100644 src/utils/runInSpinner.js delete mode 100644 test/test.js create mode 100644 test/utils/cleanReportName.spec.js create mode 100644 test/utils/cloneGITRepository.spec.js create mode 100644 test/utils/formatNpmPackages.spec.js diff --git a/README.md b/README.md index cc6bc24..39605ad 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,8 @@ This uses the official NodeSecure [runtime configuration](https://github.com/Nod "i18n": "english", "strategy": "npm", "report": { + "title": "NodeSecure Security Report", + "logoUrl": "https://avatars.githubusercontent.com/u/85318671?s=200&v=4", "theme": "light", "includeTransitiveInternal": false, "reporters": ["html", "pdf"], @@ -83,7 +85,9 @@ This uses the official NodeSecure [runtime configuration](https://github.com/Nod }, "git": { "organizationUrl": "https://github.com/NodeSecure", - "repositories": [] + "repositories": [ + "vulnera" + ] }, "charts": [ { @@ -110,9 +114,7 @@ This uses the official NodeSecure [runtime configuration](https://github.com/Nod "type": "horizontalBar", "interpolation": "d3.interpolateSinebow" } - ], - "title": "NodeSecure Security Report", - "logoUrl": "https://avatars.githubusercontent.com/u/85318671?s=200&v=4" + ] } } ``` diff --git a/package.json b/package.json index e0dc16d..5d30437 100644 --- a/package.json +++ b/package.json @@ -13,10 +13,10 @@ } }, "scripts": { - "start": "node -r dotenv/config index.js", "prepublishOnly": "pkg-ok", - "lint": "eslint index.js", - "test": "jest --coverage" + "lint": "eslint src test", + "test-only": "glob -c \"node --test-reporter=spec --test\" \"./test/**/*.spec.js\"", + "test": "c8 --all --src ./src -r html npm run test-only" }, "repository": { "type": "git", @@ -44,7 +44,6 @@ "@nodesecure/utils": "^2.2.0", "@openally/mutex": "^1.0.0", "@topcli/spinner": "^2.1.2", - "dotenv": "^16.4.5", "esbuild": "^0.22.0", "filenamify": "^6.0.0", "frequency-set": "^1.0.2", @@ -56,9 +55,10 @@ }, "devDependencies": { "@nodesecure/eslint-config": "^1.9.0", - "cross-env": "^7.0.3", + "@types/node": "^20.14.10", + "c8": "^10.1.2", "eslint": "^9.6.0", - "jest": "^29.7.0", + "glob": "^10.4.3", "pkg-ok": "^3.0.0" }, "engines": { diff --git a/src/analysis/fetch.js b/src/analysis/fetch.js index 5e8f3af..f062f27 100644 --- a/src/analysis/fetch.js +++ b/src/analysis/fetch.js @@ -1,27 +1,15 @@ +// Import Node.js Dependencies +import path from "node:path"; + // Import Third-party Dependencies import kleur from "kleur"; // Import Internal Dependencies -import { cloneGITRepository } from "../utils.js"; import { buildStatsFromNsecurePayloads } from "./extractScannerData.js"; import * as scanner from "./scanner.js"; import * as localStorage from "../localStorage.js"; -import * as utils from "../utils.js"; - -function formatNpmPackages(organizationPrefix, packages) { - if (organizationPrefix === "") { - return packages; - } - - return packages.map((pkg) => { - // in case the user has already added the organization prefix - if (pkg.startsWith(organizationPrefix)) { - return pkg; - } - - return `${organizationPrefix}/${pkg}`; - }); -} +import * as utils from "../utils/index.js"; +import * as CONSTANTS from "../constants.js"; export async function fetchPackagesAndRepositoriesData() { const config = localStorage.getConfig().report; @@ -35,11 +23,21 @@ export async function fetchPackagesAndRepositoriesData() { } const pkgStats = fetchNpm ? - await fetchPackagesStats(formatNpmPackages(config.npm.organizationPrefix, config.npm.packages)) : null; + await fetchPackagesStats( + utils.formatNpmPackages( + config.npm.organizationPrefix, + config.npm.packages + ) + ) : + null; const { repositories, organizationUrl } = config.git; const repoStats = fetchGit ? - await fetchRepositoriesStats(repositories, organizationUrl) : null; + await fetchRepositoriesStats( + repositories, + organizationUrl + ) : + null; return { pkgStats, repoStats }; } @@ -66,7 +64,12 @@ async function fetchRepositoriesStats(repositories, organizationUrl) { }, async(spinner) => { const repos = await Promise.all( - repositories.map((repositoryName) => cloneGITRepository(repositoryName, organizationUrl)) + repositories.map((repositoryName) => { + return utils.cloneGITRepository( + path.join(CONSTANTS.DIRS.CLONES, repositoryName), + `${organizationUrl}/${repositoryName}.git` + ); + }) ); spinner.text = "Fetching repositories metadata on the NPM Registry"; diff --git a/src/reporting/html.js b/src/reporting/html.js index dc77282..d5e0167 100644 --- a/src/reporting/html.js +++ b/src/reporting/html.js @@ -10,7 +10,7 @@ import kleur from "kleur"; import { taggedString } from "@nodesecure/utils"; // Import Internal Dependencies -import { cleanReportName } from "../utils.js"; +import * as utils from "../utils/index.js"; import * as CONSTANTS from "../constants.js"; import * as localStorage from "../localStorage.js"; @@ -50,7 +50,10 @@ export async function HTML(data, reportOptions = null) { const HTMLReport = kTemplateGenerator(templatePayload) .concat(`\n`); - const reportHTMLPath = path.join(CONSTANTS.DIRS.REPORTS, cleanReportName(config.title, ".html")); + const reportHTMLPath = path.join( + CONSTANTS.DIRS.REPORTS, + utils.cleanReportName(config.title, ".html") + ); await fs.writeFile(reportHTMLPath, HTMLReport); spinner.text = kleur.yellow().bold("Bundling assets with esbuild"); diff --git a/src/reporting/index.js b/src/reporting/index.js index 063c241..04e4ccf 100644 --- a/src/reporting/index.js +++ b/src/reporting/index.js @@ -2,7 +2,7 @@ import kleur from "kleur"; // Import Internal Dependencies -import * as utils from "../utils.js"; +import * as utils from "../utils/index.js"; import * as localStorage from "../localStorage.js"; // Import Reporters diff --git a/src/reporting/pdf.js b/src/reporting/pdf.js index 644c2fb..f03b2e7 100644 --- a/src/reporting/pdf.js +++ b/src/reporting/pdf.js @@ -6,21 +6,24 @@ import puppeteer from "puppeteer"; // Import Internal Dependencies import * as CONSTANTS from "../constants.js"; -import * as utils from "../utils.js"; +import * as utils from "../utils/index.js"; -export async function PDF(reportHTMLPath, options) { +export async function PDF( + reportHTMLPath, + options +) { const { title, saveOnDisk = true } = options; const browser = await puppeteer.launch({ args: ["--no-sandbox", "--disable-setuid-sandbox"] }); + const page = await browser.newPage(); try { - const page = await browser.newPage(); await page.emulateMediaType("print"); await page.goto(`file:${reportHTMLPath}`, { waitUntil: "networkidle0", - timeout: 60_000 + timeout: 20_000 }); const pdfBuffer = await page.pdf({ @@ -35,6 +38,7 @@ export async function PDF(reportHTMLPath, options) { return saveOnDisk ? path : pdfBuffer; } finally { + await page.close(); await browser.close(); } } diff --git a/src/utils.js b/src/utils.js deleted file mode 100644 index ace05e9..0000000 --- a/src/utils.js +++ /dev/null @@ -1,69 +0,0 @@ -// Import Node.js Dependencies -import path from "node:path"; -import fs from "node:fs"; - -// Import Third-party Dependencies -import git from "isomorphic-git"; -import http from "isomorphic-git/http/node/index.js"; -import filenamify from "filenamify"; -import { Spinner } from "@topcli/spinner"; -import kleur from "kleur"; - -// Import Internal Dependencies -import * as CONSTANTS from "./constants.js"; - -/** - * @async - * @function cloneGITRepository - * @description clone a given repository from github - * @param {!string} repositoryName - * @param {!string} organizationUrl - * @returns {Promise} - */ -export async function cloneGITRepository(repositoryName, organizationUrl) { - const dir = path.join(CONSTANTS.DIRS.CLONES, repositoryName); - const url = `${organizationUrl}/${repositoryName}.git`; - - await git.clone({ - fs, http, dir, url, token: process.env.GIT_TOKEN, singleBranch: true, oauth2format: "github" - }); - - return dir; -} - -/** - * @function cleanReportName - * @description clean the report name - * @param {!string} name - * @param {string} [format=null] - * @returns {string} - */ -export function cleanReportName(name, format = null) { - const cleanName = filenamify(name); - if (format === null) { - return cleanName; - } - - return path.extname(cleanName) === format ? cleanName : `${cleanName}${format}`; -} - -export async function runInSpinner(options, asyncHandler) { - const { title, start = void 0 } = options; - - const spinner = new Spinner() - .start(start, { withPrefix: `${kleur.gray().bold(title)} - ` }); - - try { - const response = await asyncHandler(spinner); - - const elapsed = `${spinner.elapsedTime.toFixed(2)}ms`; - spinner.succeed(kleur.white().bold(`successfully executed in ${kleur.green().bold(elapsed)}`)); - - return response; - } - catch (err) { - spinner.failed(err.message); - - throw err; - } -} diff --git a/src/utils/cleanReportName.js b/src/utils/cleanReportName.js new file mode 100644 index 0000000..be5be39 --- /dev/null +++ b/src/utils/cleanReportName.js @@ -0,0 +1,26 @@ +// Import Node.js Dependencies +import path from "node:path"; + +// Import Third-party Dependencies +import filenamify from "filenamify"; + +/** + * @function cleanReportName + * @description clean the report name + * @param {!string} name + * @param {string} [extension=null] + * @returns {string} + */ +export function cleanReportName( + name, + extension = null +) { + const cleanName = filenamify(name); + if (extension === null) { + return cleanName; + } + + return path.extname(cleanName) === extension ? + cleanName : + `${cleanName}${extension}`; +} diff --git a/src/utils/cloneGITRepository.js b/src/utils/cloneGITRepository.js new file mode 100644 index 0000000..c348f1a --- /dev/null +++ b/src/utils/cloneGITRepository.js @@ -0,0 +1,32 @@ +// Import Node.js Dependencies +import fs from "node:fs"; + +// Import Third-party Dependencies +import git from "isomorphic-git"; +import http from "isomorphic-git/http/node/index.js"; + +/** + * @async + * @function cloneGITRepository + * @description clone a given repository from github + * @param {!string} dir + * @param {!string} url + * + * @returns {Promise} + */ +export async function cloneGITRepository( + dir, + url +) { + await git.clone({ + fs, + http, + dir, + url, + token: process.env.GIT_TOKEN, + singleBranch: true, + oauth2format: "github" + }); + + return dir; +} diff --git a/src/utils/formatNpmPackages.js b/src/utils/formatNpmPackages.js new file mode 100644 index 0000000..1385b76 --- /dev/null +++ b/src/utils/formatNpmPackages.js @@ -0,0 +1,21 @@ +/** + * @param {!string} organizationPrefix + * @param {string[]} packages + * + * @returns {string[]} + */ +export function formatNpmPackages( + organizationPrefix, + packages +) { + if (organizationPrefix === "") { + return packages; + } + + return packages.map((pkg) => { + // in case the user has already added the organization prefix + return pkg.startsWith(organizationPrefix) ? + pkg : + `${organizationPrefix}/${pkg}`; + }); +} diff --git a/src/utils/index.js b/src/utils/index.js new file mode 100644 index 0000000..310f93b --- /dev/null +++ b/src/utils/index.js @@ -0,0 +1,4 @@ +export * from "./cleanReportName.js"; +export * from "./cloneGITRepository.js"; +export * from "./runInSpinner.js"; +export * from "./formatNpmPackages.js"; diff --git a/src/utils/runInSpinner.js b/src/utils/runInSpinner.js new file mode 100644 index 0000000..a27ad78 --- /dev/null +++ b/src/utils/runInSpinner.js @@ -0,0 +1,27 @@ +// Import Third-party Dependencies +import { Spinner } from "@topcli/spinner"; +import kleur from "kleur"; + +export async function runInSpinner( + options, + asyncHandler +) { + const { title, start = void 0 } = options; + + const spinner = new Spinner() + .start(start, { withPrefix: `${kleur.gray().bold(title)} - ` }); + + try { + const response = await asyncHandler(spinner); + + const elapsed = `${spinner.elapsedTime.toFixed(2)}ms`; + spinner.succeed(kleur.white().bold(`successfully executed in ${kleur.green().bold(elapsed)}`)); + + return response; + } + catch (err) { + spinner.failed(err.message); + + throw err; + } +} diff --git a/test/test.js b/test/test.js deleted file mode 100644 index 0bee62c..0000000 --- a/test/test.js +++ /dev/null @@ -1,3 +0,0 @@ -test("husky should pass test", () => { - expect(1).toStrictEqual(1); -}); diff --git a/test/utils/cleanReportName.spec.js b/test/utils/cleanReportName.spec.js new file mode 100644 index 0000000..d164f1e --- /dev/null +++ b/test/utils/cleanReportName.spec.js @@ -0,0 +1,26 @@ +// Import Node.js Dependencies +import { describe, it } from "node:test"; +import assert from "node:assert"; + +// Import Internal Dependencies +import { cleanReportName } from "../../src/utils/index.js"; + +describe("cleanReportName", () => { + it("should remove invalid Windows characters", () => { + const invalidStr = ""; + + assert.strictEqual( + cleanReportName(invalidStr), + "!foo!bar!" + ); + }); + + it("should add the extension if it's missing from the input", () => { + const fileName = "foo*bar"; + + assert.strictEqual( + cleanReportName(fileName, ".png"), + "foo!bar.png" + ); + }); +}); diff --git a/test/utils/cloneGITRepository.spec.js b/test/utils/cloneGITRepository.spec.js new file mode 100644 index 0000000..543851e --- /dev/null +++ b/test/utils/cloneGITRepository.spec.js @@ -0,0 +1,34 @@ +// Import Node.js Dependencies +import { describe, it } from "node:test"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import assert from "node:assert"; + +// Import Internal Dependencies +import { cloneGITRepository } from "../../src/utils/index.js"; + +describe("cloneGITRepository", () => { + it("should clone a remote GIT repository", async() => { + const dest = await fs.mkdtemp( + path.join(os.tmpdir(), "nsecure-report-git-") + ); + + try { + const dir = await cloneGITRepository( + dest, + "https://github.com/NodeSecure/Governance.git" + ); + + assert.strictEqual(dir, dest); + const files = (await fs.readdir(dest, { withFileTypes: true })) + .flatMap((dirent) => dirent.isFile() ? [dirent.name] : []); + + assert.ok(files.includes("CODE_OF_CONDUCT.md")); + assert.ok(files.includes("README.md")); + } + finally { + await fs.rm(dest, { force: true, recursive: true }); + } + }); +}); diff --git a/test/utils/formatNpmPackages.spec.js b/test/utils/formatNpmPackages.spec.js new file mode 100644 index 0000000..7ac711a --- /dev/null +++ b/test/utils/formatNpmPackages.spec.js @@ -0,0 +1,41 @@ +// Import Node.js Dependencies +import { describe, test } from "node:test"; +import assert from "node:assert"; + +// Import Internal Dependencies +import { formatNpmPackages } from "../../src/utils/index.js"; + +describe("formatNpmPackages", () => { + test("If no organizationPrefix is provided, it should return the packages list as is.", () => { + const packages = [ + "@nodesecure/js-x-ray" + ]; + const formatedPackages = formatNpmPackages("", packages); + + assert.strictEqual( + formatedPackages, + packages + ); + }); + + test(`Given an organizationPrefix, it must add the prefix to the packages where it is missing + and ignore those who already have the prefix.`, () => { + const packages = [ + "@nodesecure/js-x-ray", + "scanner" + ]; + const formatedPackages = formatNpmPackages("@nodesecure", packages); + + assert.notStrictEqual( + formatedPackages, + packages + ); + assert.deepEqual( + formatedPackages, + [ + "@nodesecure/js-x-ray", + "@nodesecure/scanner" + ] + ); + }); +});