From 58b39d942245f1f307a8c986158432499731017f Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Tue, 4 Feb 2025 15:44:36 -0500 Subject: [PATCH 01/13] add message check workflow and script Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/README.md | 11 ++ .dependency/zwe_message_checks/index.js | 157 ++++++++++++++++++ .../zwe_message_checks/package-lock.json | 67 ++++++++ .dependency/zwe_message_checks/package.json | 15 ++ .github/workflows/zwe-message-checks.yml | 39 +++++ .gitignore | 2 + 6 files changed, 291 insertions(+) create mode 100644 .dependency/zwe_message_checks/README.md create mode 100644 .dependency/zwe_message_checks/index.js create mode 100644 .dependency/zwe_message_checks/package-lock.json create mode 100644 .dependency/zwe_message_checks/package.json create mode 100644 .github/workflows/zwe-message-checks.yml diff --git a/.dependency/zwe_message_checks/README.md b/.dependency/zwe_message_checks/README.md new file mode 100644 index 0000000000..18546b508a --- /dev/null +++ b/.dependency/zwe_message_checks/README.md @@ -0,0 +1,11 @@ +# ZWE CLI Message Checks + +Run `npm install` in this directory and then `node index.js` to scan error messages defined in the ZWE command line and error messages used by the ZWE tool, whether in shell scripts or typescript source. + +The tool leverages some code in the [Zowe Doc Generation Automation](../zwe_doc_generation/). It will output multiple evaluations of our message use within ZWE, including unused messages, mismatched message IDs and contents, and disparities between message definitions and their use in ZWE sources. + +This is not 100% accurate in all cases, particularly when comparing message content, as the capture of message content from the sources is simplistic and therefore incomplete, but it is a decent starting point. If message content capture in the sources improves, the accuracy of the tool can improve with it. Alternatively, we may prefer a design where sources pull messages from a common library based on the message definitions, avoiding the need to check their accuracy in source code altogether. + +If the tool finds errors it is confident in, it will return quit with exitCode=1, which should trigger a failure in Github Actions. + +TODO: use core.setFailed diff --git a/.dependency/zwe_message_checks/index.js b/.dependency/zwe_message_checks/index.js new file mode 100644 index 0000000000..f217eec67d --- /dev/null +++ b/.dependency/zwe_message_checks/index.js @@ -0,0 +1,157 @@ +/** + * This program and the accompanying materials are made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-v20.html + * + * SPDX-License-Identifier: EPL-2.0 + * + * Copyright IBM Corporation 2021 + */ +const fs = require('fs-extra'); +const path = require('path'); +const sc = require('string-comparison'); +const { getDocumentationTree } = require('../zwe_doc_generation/doc-tree'); + +const zweRootDir = path.join(__dirname, '../../bin'); +const rootDocNode = getDocumentationTree({ dir: path.join(zweRootDir, 'commands'), command: 'zwe' }); + +let statusFailed = false; +const discoveredMsgs = []; // filled out by getMessagesUsedByImplementations() + +// inspect sources in two dirs: commands and libs. we miss zwe itself and code exceptions for it +const dirs = [path.join(zweRootDir, 'commands'), path.join(zweRootDir, 'libs')]; +for (const dir of dirs) { + discoveredMsgs.push(...getMessagesUsedByImplementations(dir)); +} + +// second, collect all message ids listed in .errors +const collectedMsgs = collectMessageIds(rootDocNode); +console.log('---- Duplicate Message Content or IDs defined in .errors ----\n'); +if (collectedMsgs?.errors?.length > 0) { + for (const error of collectedMsgs.errors) { + console.log(error.message); + } +} +console.log('') + +const flatExpectedMessages = collectedMsgs.messages.map((msg) => msg.id); +const msgTally = {}; +for (const msg of flatExpectedMessages) { + msgTally[msg] = {count: 0}; +} + +console.log('---- Messages Used and Not Defined in .errors ----'); +for(const msgSpec of discoveredMsgs) { + for(const msg of msgSpec.messages) { + if (!flatExpectedMessages.includes(msg.messageId)) { + console.log(`Missing message: ${JSON.stringify(msg)}`); + statusFailed = true; + continue; + } + msgTally[msg.messageId].count++ + } +} +console.log('') +console.log('---- Unused Messages defined in .errors ----'); +for(const msgId of Object.keys(msgTally)) { + if (msgTally[msgId].count === 0 && msgId !== 'ZWEL0103E') { // ZWEL0103E is in 'zwe', which isn't scanned + console.log('Unused message: ' + JSON.stringify(msgId)); + statusFailed = true; + } +} +console.log() +// this will not set 'statusFailed' since the results may not be accurate. +// toggling the similarity threshold greatly impacts output... setting the threshold lower (closer to 0) suppresses +// output volume, while setting it higher (closer to 1) will display more messages in the log +console.log('---- Experimental: Messages whose content differs from the definition in .errors ----'); +const similarityExceptions = ['The password for data set storing importing certificate (zowe.setup.certificate.keyring.import.password) is not defined in Zowe YAML configuration file.'] +for(const msgSpec of discoveredMsgs) { + for(const msg of msgSpec.messages) { + const errorDef = collectedMsgs.messages.find((item) => item.id === msg.messageId); + // lets only examine message contents where we have more than a few characters cut off by a newline + if (errorDef && msg.message.length > 15 && !similarityExceptions.includes(msg.message)) { + const similarity = sc.default.levenshtein.similarity(msg.message, errorDef.message); + if (similarity < 0.35) { + console.log(`${msg.message} VERSUS ${errorDef.message} --- ${msg.messageId} VERSUS ${errorDef.id}`); + } + } + + } + console.log() + +} + +if (statusFailed) { + process.exit(1); +} + +function collectMessageIds(docNode) { + + const messages = []; + const errors = []; + if (docNode?.children?.length > 0) { + for (const child of docNode.children) { + const recursedResult = collectMessageIds(child); + messages.push(...recursedResult.messages); + errors.push(...recursedResult.errors); + } + } + const errorsFile = docNode?.['.errors'] + if (errorsFile) { + fs.readFileSync(errorsFile, 'utf8').split('\n').forEach((line) => { + const pieces = line.trim().split('|'); + if (pieces.length > 0 && pieces[0].trim().length > 0) { + // check for duplicates + // reconstruct full message string, in case it contained | characters + const originalMsg = pieces.slice(2).join('|'); + const matchingMsgId = messages.find((item) => item.id === pieces[0] && item.message !== originalMsg); + const matchingMsgContent = messages.find((item) => item.message === pieces[2] && item.id !== pieces[0]); + if (matchingMsgId) { + errors.push({ type: 'ID', message: `Dup ID: |${pieces[0]}:${originalMsg}| VERSUS |${matchingMsgId.id}:${matchingMsgId.message}|`}); + } + if (matchingMsgContent) { + errors.push({ type: 'MSG', message: `Dup MSG: |${pieces[0]}:${originalMsg}| VERSUS |${matchingMsgContent.id}:${matchingMsgContent.message}|`}) + } + messages.push({ id: pieces[0], message: originalMsg }); + } + }) + } + return { messages: messages, errors: errors}; + +} + +function getMessagesUsedByImplementations(zweDir) { + + const messages = []; + + if (!fs.existsSync(zweDir) && !fs.lstatSync(zweDir).isDirectory()) { + throw new Error('Bad directory passed to zwe message checks: '+zweDir); + } + + const files = fs.readdirSync(zweDir); + const dirs = files.filter((file) => fs.statSync(path.join(zweDir, file)).isDirectory()); + const srcFiles = files.filter((file) => file.endsWith('.ts') || file.endsWith('.sh') || file.endsWith('zwe')); + dirs.forEach((dir) => + messages.push(...getMessagesUsedByImplementations(path.join(zweDir, dir)))); + for(const src of srcFiles) { + // find messages matching ZWELXXX + const srcFile = path.join(zweDir, src); + const content = fs.readFileSync(srcFile, 'utf8'); + const matches = content.matchAll(/(ZWEL\d{4}[EIDTW])(.*?)["'`]/gm); + + for (const match of matches) { + const message = match[2].replaceAll(/\${.*?}/gm,'%s'); + if (!messages.includes(message)) { + const leafDir = path.basename(path.dirname(srcFile)); + const existing = messages.find((item) => item.src === srcFile); + if (existing) { + existing.messages.push({messageId: match[1], message: message.substring(1).trim()}); + } else { + messages.push({ command: leafDir, src: srcFile, messages: [{messageId: match[1], message: message.substring(1).trim() }]}); + } + } + } + } + return messages; +} + diff --git a/.dependency/zwe_message_checks/package-lock.json b/.dependency/zwe_message_checks/package-lock.json new file mode 100644 index 0000000000..e35119fb72 --- /dev/null +++ b/.dependency/zwe_message_checks/package-lock.json @@ -0,0 +1,67 @@ +{ + "name": "zwe_message_checks", + "version": "0.0.1", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "zwe_message_checks", + "version": "0.0.1", + "license": "EPL-2.0", + "devDependencies": { + "fs-extra": "^11.3.0", + "string-comparison": "^1.3.0" + } + }, + "node_modules/fs-extra": { + "version": "11.3.0", + "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-11.3.0.tgz", + "integrity": "sha512-Z4XaCL6dUDHfP/jT25jJKMmtxvuwbkrD1vNSMFlo9lNLY2c5FHYSQgHPRZUjAB26TpDEoW9HCOgplrdbaPV/ew==", + "dev": true, + "dependencies": { + "graceful-fs": "^4.2.0", + "jsonfile": "^6.0.1", + "universalify": "^2.0.0" + }, + "engines": { + "node": ">=14.14" + } + }, + "node_modules/graceful-fs": { + "version": "4.2.11", + "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.11.tgz", + "integrity": "sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==", + "dev": true + }, + "node_modules/jsonfile": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-6.1.0.tgz", + "integrity": "sha512-5dgndWOriYSm5cnYaJNhalLNDKOqFwyDB/rr1E9ZsGciGvKPs8R2xYGCacuf3z6K1YKDz182fd+fY3cn3pMqXQ==", + "dev": true, + "dependencies": { + "universalify": "^2.0.0" + }, + "optionalDependencies": { + "graceful-fs": "^4.1.6" + } + }, + "node_modules/string-comparison": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/string-comparison/-/string-comparison-1.3.0.tgz", + "integrity": "sha512-46aD+slEwybxAMPRII83ATbgMgTiz5P8mVd7Z6VJsCzSHFjdt1hkAVLeFxPIyEb11tc6ihpJTlIqoO0MCF6NPw==", + "dev": true, + "engines": { + "node": "^16.0.0 || >=18.0.0" + } + }, + "node_modules/universalify": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/universalify/-/universalify-2.0.1.tgz", + "integrity": "sha512-gptHNQghINnc/vTGIk0SOFGFNXw7JVrlRUtConJRlvaw6DuX0wO5Jeko9sWrMBhh+PsYAZ7oXAiOnf/UKogyiw==", + "dev": true, + "engines": { + "node": ">= 10.0.0" + } + } + } +} diff --git a/.dependency/zwe_message_checks/package.json b/.dependency/zwe_message_checks/package.json new file mode 100644 index 0000000000..de0733ad40 --- /dev/null +++ b/.dependency/zwe_message_checks/package.json @@ -0,0 +1,15 @@ +{ + "name": "zwe_message_checks", + "version": "0.0.1", + "description": "Analyzes uses of messages within ZWE, looking for duplicates, untracked, and unused message IDs.", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "EPL-2.0", + "devDependencies": { + "fs-extra": "^11.3.0", + "string-comparison": "^1.3.0" + } +} diff --git a/.github/workflows/zwe-message-checks.yml b/.github/workflows/zwe-message-checks.yml new file mode 100644 index 0000000000..531997ac48 --- /dev/null +++ b/.github/workflows/zwe-message-checks.yml @@ -0,0 +1,39 @@ +name: Update zwe documentation + +on: + + pull_request: + types: [opened, synchronize] + workflow_dispatch: + +env: + ZWE_MESSAGE_CHECKS_DIR: .dependency/zwe_message_checks + +jobs: + update-zwe-documentation: + name: Update zwe documentation on docs-site + runs-on: ubuntu-latest + + steps: + - name: Set up Node + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up git + run: | + git config --global user.email "zowe-robot@users.noreply.github.com" + git config --global user.name "Zowe Robot" + git config --global pull.rebase false # configure to merge in changes from remote branches + + - name: Prepare node project + working-directory: ${{ env.ZWE_MESSAGE_CHECKS_DIR }} + run: npm install + + - name: Check zwe messages for issues and print them to the log + working-directory: ${{ env.ZWE_MESSAGE_CHECKS_DIR }} + id: duplicates + run: node ${{ env.ZWE_MESSAGE_CHECKS_DIR }}/index.js diff --git a/.gitignore b/.gitignore index 9ca12300d3..5210b85b15 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,8 @@ tmp/ # Compiled TS files bin/libs/*.js build/zwe/out +bin/commands/**/*.js +bin/utils/ObjUtils.js # Mac files .DS_Store From 0884f0becf56b33a230b24e67412d8911f642ac4 Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Tue, 4 Feb 2025 16:06:01 -0500 Subject: [PATCH 02/13] update workflow title Signed-off-by: MarkAckert --- .github/workflows/zwe-message-checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/zwe-message-checks.yml b/.github/workflows/zwe-message-checks.yml index 531997ac48..e3cf9dc240 100644 --- a/.github/workflows/zwe-message-checks.yml +++ b/.github/workflows/zwe-message-checks.yml @@ -1,4 +1,4 @@ -name: Update zwe documentation +name: ZWE Message Analysis on: From 038ed5db1b080f50077262cdebed93bae5a95bff Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Tue, 4 Feb 2025 16:07:24 -0500 Subject: [PATCH 03/13] fix some more job and step names Signed-off-by: MarkAckert --- .github/workflows/zwe-message-checks.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/zwe-message-checks.yml b/.github/workflows/zwe-message-checks.yml index e3cf9dc240..c9abdd8eb5 100644 --- a/.github/workflows/zwe-message-checks.yml +++ b/.github/workflows/zwe-message-checks.yml @@ -10,8 +10,8 @@ env: ZWE_MESSAGE_CHECKS_DIR: .dependency/zwe_message_checks jobs: - update-zwe-documentation: - name: Update zwe documentation on docs-site + run-tests: + name: Run the ZWE Message Analysis runs-on: ubuntu-latest steps: From 0c4a89a26605f48741de9db53394ffaeaf2a2a2d Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Tue, 4 Feb 2025 16:11:02 -0500 Subject: [PATCH 04/13] wrong working dir Signed-off-by: MarkAckert --- .github/workflows/zwe-message-checks.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/zwe-message-checks.yml b/.github/workflows/zwe-message-checks.yml index c9abdd8eb5..d5cf1a5fc8 100644 --- a/.github/workflows/zwe-message-checks.yml +++ b/.github/workflows/zwe-message-checks.yml @@ -34,6 +34,5 @@ jobs: run: npm install - name: Check zwe messages for issues and print them to the log - working-directory: ${{ env.ZWE_MESSAGE_CHECKS_DIR }} id: duplicates run: node ${{ env.ZWE_MESSAGE_CHECKS_DIR }}/index.js From 412ed4be327a47a9e6460c83c219fc66ccd09807 Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Tue, 4 Feb 2025 16:13:32 -0500 Subject: [PATCH 05/13] fix whitespace issue Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/index.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.dependency/zwe_message_checks/index.js b/.dependency/zwe_message_checks/index.js index f217eec67d..ecd32e3606 100644 --- a/.dependency/zwe_message_checks/index.js +++ b/.dependency/zwe_message_checks/index.js @@ -69,7 +69,7 @@ for(const msgSpec of discoveredMsgs) { for(const msg of msgSpec.messages) { const errorDef = collectedMsgs.messages.find((item) => item.id === msg.messageId); // lets only examine message contents where we have more than a few characters cut off by a newline - if (errorDef && msg.message.length > 15 && !similarityExceptions.includes(msg.message)) { + if (errorDef?.message && msg.message.length > 15 && !similarityExceptions.includes(msg.message)) { const similarity = sc.default.levenshtein.similarity(msg.message, errorDef.message); if (similarity < 0.35) { console.log(`${msg.message} VERSUS ${errorDef.message} --- ${msg.messageId} VERSUS ${errorDef.id}`); @@ -77,9 +77,8 @@ for(const msgSpec of discoveredMsgs) { } } - console.log() - } +console.log() if (statusFailed) { process.exit(1); From 6e8740ca365818bed4f3d109a418a07713338e3f Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Wed, 5 Feb 2025 10:00:02 -0500 Subject: [PATCH 06/13] add statusFailed to dup IDs, add info to output on where issues were detected Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/index.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/.dependency/zwe_message_checks/index.js b/.dependency/zwe_message_checks/index.js index ecd32e3606..f758a8ba51 100644 --- a/.dependency/zwe_message_checks/index.js +++ b/.dependency/zwe_message_checks/index.js @@ -28,6 +28,7 @@ for (const dir of dirs) { const collectedMsgs = collectMessageIds(rootDocNode); console.log('---- Duplicate Message Content or IDs defined in .errors ----\n'); if (collectedMsgs?.errors?.length > 0) { + statusFailed = true; for (const error of collectedMsgs.errors) { console.log(error.message); } @@ -55,7 +56,8 @@ console.log('') console.log('---- Unused Messages defined in .errors ----'); for(const msgId of Object.keys(msgTally)) { if (msgTally[msgId].count === 0 && msgId !== 'ZWEL0103E') { // ZWEL0103E is in 'zwe', which isn't scanned - console.log('Unused message: ' + JSON.stringify(msgId)); + const definition = collectedMsgs.messages.find((it) => it.id === msgId); + console.log(`Unused message: ${msgId} [${definition.source}]`); statusFailed = true; } } @@ -72,7 +74,7 @@ for(const msgSpec of discoveredMsgs) { if (errorDef?.message && msg.message.length > 15 && !similarityExceptions.includes(msg.message)) { const similarity = sc.default.levenshtein.similarity(msg.message, errorDef.message); if (similarity < 0.35) { - console.log(`${msg.message} VERSUS ${errorDef.message} --- ${msg.messageId} VERSUS ${errorDef.id}`); + console.log(`${msg.messageId}:${msg.message}[${msgSpec.src}] VERSUS ${errorDef.id}:${errorDef.message}[${errorDef.source}]`); } } @@ -98,6 +100,7 @@ function collectMessageIds(docNode) { const errorsFile = docNode?.['.errors'] if (errorsFile) { fs.readFileSync(errorsFile, 'utf8').split('\n').forEach((line) => { + const shortErrorsPath = errorsFile.split('zowe-install-packaging/')[1]; const pieces = line.trim().split('|'); if (pieces.length > 0 && pieces[0].trim().length > 0) { // check for duplicates @@ -106,12 +109,12 @@ function collectMessageIds(docNode) { const matchingMsgId = messages.find((item) => item.id === pieces[0] && item.message !== originalMsg); const matchingMsgContent = messages.find((item) => item.message === pieces[2] && item.id !== pieces[0]); if (matchingMsgId) { - errors.push({ type: 'ID', message: `Dup ID: |${pieces[0]}:${originalMsg}| VERSUS |${matchingMsgId.id}:${matchingMsgId.message}|`}); + errors.push({ type: 'ID', message: `Dup ID: |${pieces[0]}:${originalMsg}[${shortErrorsPath}]| VERSUS |${matchingMsgId.id}:${matchingMsgId.message}[${matchingMsgId.source}]|`}); } if (matchingMsgContent) { - errors.push({ type: 'MSG', message: `Dup MSG: |${pieces[0]}:${originalMsg}| VERSUS |${matchingMsgContent.id}:${matchingMsgContent.message}|`}) + errors.push({ type: 'MSG', message: `Dup MSG: |${pieces[0]}:${originalMsg}[${shortErrorsPath}]| VERSUS |${matchingMsgContent.id}:${matchingMsgContent.message}[${matchingMsgContent.source}]|`}) } - messages.push({ id: pieces[0], message: originalMsg }); + messages.push({ id: pieces[0], message: originalMsg, source: shortErrorsPath }); } }) } @@ -135,6 +138,7 @@ function getMessagesUsedByImplementations(zweDir) { for(const src of srcFiles) { // find messages matching ZWELXXX const srcFile = path.join(zweDir, src); + const srcFileShort = srcFile.split('zowe-install-packaging/')[1]; const content = fs.readFileSync(srcFile, 'utf8'); const matches = content.matchAll(/(ZWEL\d{4}[EIDTW])(.*?)["'`]/gm); @@ -142,11 +146,11 @@ function getMessagesUsedByImplementations(zweDir) { const message = match[2].replaceAll(/\${.*?}/gm,'%s'); if (!messages.includes(message)) { const leafDir = path.basename(path.dirname(srcFile)); - const existing = messages.find((item) => item.src === srcFile); + const existing = messages.find((item) => item.src === srcFileShort); if (existing) { existing.messages.push({messageId: match[1], message: message.substring(1).trim()}); } else { - messages.push({ command: leafDir, src: srcFile, messages: [{messageId: match[1], message: message.substring(1).trim() }]}); + messages.push({ command: leafDir, src: srcFileShort, messages: [{messageId: match[1], message: message.substring(1).trim() }]}); } } } From 49e751ba1af2159e7e29a2f3e543e3d9c1322c7e Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Wed, 5 Feb 2025 10:03:45 -0500 Subject: [PATCH 07/13] change split point - not being picked up in gha Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.dependency/zwe_message_checks/index.js b/.dependency/zwe_message_checks/index.js index f758a8ba51..e6bf104d58 100644 --- a/.dependency/zwe_message_checks/index.js +++ b/.dependency/zwe_message_checks/index.js @@ -100,7 +100,7 @@ function collectMessageIds(docNode) { const errorsFile = docNode?.['.errors'] if (errorsFile) { fs.readFileSync(errorsFile, 'utf8').split('\n').forEach((line) => { - const shortErrorsPath = errorsFile.split('zowe-install-packaging/')[1]; + const shortErrorsPath = 'bin/'+errorsFile.split('bin/')[1]; const pieces = line.trim().split('|'); if (pieces.length > 0 && pieces[0].trim().length > 0) { // check for duplicates @@ -138,7 +138,7 @@ function getMessagesUsedByImplementations(zweDir) { for(const src of srcFiles) { // find messages matching ZWELXXX const srcFile = path.join(zweDir, src); - const srcFileShort = srcFile.split('zowe-install-packaging/')[1]; + const srcFileShort = 'bin/'+srcFile.split('bin/')[1]; const content = fs.readFileSync(srcFile, 'utf8'); const matches = content.matchAll(/(ZWEL\d{4}[EIDTW])(.*?)["'`]/gm); From 285f5c8377dd69bdc5616c097c62eeae7e46edf6 Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Wed, 5 Feb 2025 10:05:41 -0500 Subject: [PATCH 08/13] add a newline to help with output readability Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.dependency/zwe_message_checks/index.js b/.dependency/zwe_message_checks/index.js index e6bf104d58..1cdbab63eb 100644 --- a/.dependency/zwe_message_checks/index.js +++ b/.dependency/zwe_message_checks/index.js @@ -74,7 +74,7 @@ for(const msgSpec of discoveredMsgs) { if (errorDef?.message && msg.message.length > 15 && !similarityExceptions.includes(msg.message)) { const similarity = sc.default.levenshtein.similarity(msg.message, errorDef.message); if (similarity < 0.35) { - console.log(`${msg.messageId}:${msg.message}[${msgSpec.src}] VERSUS ${errorDef.id}:${errorDef.message}[${errorDef.source}]`); + console.log(`${msg.messageId}:${msg.message}[${msgSpec.src}] VERSUS ${errorDef.id}:${errorDef.message}[${errorDef.source}]\n`); } } From 1f8e1b192de14fe131dbbebb81984a889e0edf66 Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Wed, 5 Feb 2025 10:09:12 -0500 Subject: [PATCH 09/13] minor edits to readme Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.dependency/zwe_message_checks/README.md b/.dependency/zwe_message_checks/README.md index 18546b508a..19bbb5d62f 100644 --- a/.dependency/zwe_message_checks/README.md +++ b/.dependency/zwe_message_checks/README.md @@ -4,8 +4,6 @@ Run `npm install` in this directory and then `node index.js` to scan error messa The tool leverages some code in the [Zowe Doc Generation Automation](../zwe_doc_generation/). It will output multiple evaluations of our message use within ZWE, including unused messages, mismatched message IDs and contents, and disparities between message definitions and their use in ZWE sources. -This is not 100% accurate in all cases, particularly when comparing message content, as the capture of message content from the sources is simplistic and therefore incomplete, but it is a decent starting point. If message content capture in the sources improves, the accuracy of the tool can improve with it. Alternatively, we may prefer a design where sources pull messages from a common library based on the message definitions, avoiding the need to check their accuracy in source code altogether. +This is not 100% accurate in all cases, particularly when comparing message content, as the capture of message content from the sources is simplistic and therefore incomplete, but it is a decent starting point. If message content capture in the sources improves, the accuracy of the tool can improve with it. Alternatively, we may prefer a design where ZWE sources pull messages from a common library based on the message definitions, avoiding the need to check their usage in source code altogether. If the tool finds errors it is confident in, it will return quit with exitCode=1, which should trigger a failure in Github Actions. - -TODO: use core.setFailed From c83c4d3fd1f055ea9073aab198c84bf34377d46f Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Wed, 5 Feb 2025 10:36:54 -0500 Subject: [PATCH 10/13] replace unix-style path separators with path.sep Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.dependency/zwe_message_checks/index.js b/.dependency/zwe_message_checks/index.js index 1cdbab63eb..7108eb2139 100644 --- a/.dependency/zwe_message_checks/index.js +++ b/.dependency/zwe_message_checks/index.js @@ -12,7 +12,7 @@ const path = require('path'); const sc = require('string-comparison'); const { getDocumentationTree } = require('../zwe_doc_generation/doc-tree'); -const zweRootDir = path.join(__dirname, '../../bin'); +const zweRootDir = path.resolve(__dirname, '..','..', 'bin'); const rootDocNode = getDocumentationTree({ dir: path.join(zweRootDir, 'commands'), command: 'zwe' }); let statusFailed = false; @@ -100,7 +100,7 @@ function collectMessageIds(docNode) { const errorsFile = docNode?.['.errors'] if (errorsFile) { fs.readFileSync(errorsFile, 'utf8').split('\n').forEach((line) => { - const shortErrorsPath = 'bin/'+errorsFile.split('bin/')[1]; + const shortErrorsPath = 'bin'+path.sep+errorsFile.split('bin'+path.sep)[1]; const pieces = line.trim().split('|'); if (pieces.length > 0 && pieces[0].trim().length > 0) { // check for duplicates @@ -138,7 +138,7 @@ function getMessagesUsedByImplementations(zweDir) { for(const src of srcFiles) { // find messages matching ZWELXXX const srcFile = path.join(zweDir, src); - const srcFileShort = 'bin/'+srcFile.split('bin/')[1]; + const srcFileShort = 'bin'+path.sep+srcFile.split('bin'+path.sep)[1]; const content = fs.readFileSync(srcFile, 'utf8'); const matches = content.matchAll(/(ZWEL\d{4}[EIDTW])(.*?)["'`]/gm); From ccdb657e2bd74e2338bee770d1232263c524dda6 Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Wed, 5 Feb 2025 11:39:59 -0500 Subject: [PATCH 11/13] improve duplicate accuracy Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/index.js | 54 ++++++++++++------- .../zwe_message_checks/package-lock.json | 7 +++ .dependency/zwe_message_checks/package.json | 1 + 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/.dependency/zwe_message_checks/index.js b/.dependency/zwe_message_checks/index.js index 7108eb2139..f12a4b70e9 100644 --- a/.dependency/zwe_message_checks/index.js +++ b/.dependency/zwe_message_checks/index.js @@ -10,6 +10,7 @@ const fs = require('fs-extra'); const path = require('path'); const sc = require('string-comparison'); +const _ = require('lodash'); const { getDocumentationTree } = require('../zwe_doc_generation/doc-tree'); const zweRootDir = path.resolve(__dirname, '..','..', 'bin'); @@ -26,16 +27,17 @@ for (const dir of dirs) { // second, collect all message ids listed in .errors const collectedMsgs = collectMessageIds(rootDocNode); +const dupErrors = findDuplicates(collectedMsgs); console.log('---- Duplicate Message Content or IDs defined in .errors ----\n'); -if (collectedMsgs?.errors?.length > 0) { +if (dupErrors.length > 0) { statusFailed = true; - for (const error of collectedMsgs.errors) { + for (const error of dupErrors) { console.log(error.message); } } console.log('') -const flatExpectedMessages = collectedMsgs.messages.map((msg) => msg.id); +const flatExpectedMessages = collectedMsgs.map((msg) => msg.id); const msgTally = {}; for (const msg of flatExpectedMessages) { msgTally[msg] = {count: 0}; @@ -56,7 +58,7 @@ console.log('') console.log('---- Unused Messages defined in .errors ----'); for(const msgId of Object.keys(msgTally)) { if (msgTally[msgId].count === 0 && msgId !== 'ZWEL0103E') { // ZWEL0103E is in 'zwe', which isn't scanned - const definition = collectedMsgs.messages.find((it) => it.id === msgId); + const definition = collectedMsgs.find((it) => it.id === msgId); console.log(`Unused message: ${msgId} [${definition.source}]`); statusFailed = true; } @@ -69,7 +71,7 @@ console.log('---- Experimental: Messages whose content differs from the definiti const similarityExceptions = ['The password for data set storing importing certificate (zowe.setup.certificate.keyring.import.password) is not defined in Zowe YAML configuration file.'] for(const msgSpec of discoveredMsgs) { for(const msg of msgSpec.messages) { - const errorDef = collectedMsgs.messages.find((item) => item.id === msg.messageId); + const errorDef = collectedMsgs.find((item) => item.id === msg.messageId); // lets only examine message contents where we have more than a few characters cut off by a newline if (errorDef?.message && msg.message.length > 15 && !similarityExceptions.includes(msg.message)) { const similarity = sc.default.levenshtein.similarity(msg.message, errorDef.message); @@ -86,39 +88,53 @@ if (statusFailed) { process.exit(1); } +function findDuplicates(collectedMsgs) { + const errors = []; + // flatten and get unique IDs + const uniqIds = _.uniq(collectedMsgs.map((it) => it.id)); + for (const id of uniqIds) { + const matchingIds = _.uniqBy(collectedMsgs.filter((it) => it.id === id), 'message') + //exclude the direct match + if (matchingIds.length > 1) { + const errorText = matchingIds.reduce((prev, curr) => prev + `|${curr.message}[${curr.source}]|`, ''); + errors.push({type: 'ID', message: `Dup ID: ${id}, ${matchingIds.length} Locations: \n${errorText}`}); + } + } + const uniqMsgs = _.uniq(collectedMsgs.map((it) => it.message)); + for (const msg of uniqMsgs) { + const matchingMsgs = _.uniqBy(collectedMsgs.filter((it) => it.message === msg), 'id') + if (matchingMsgs.length > 1) { + const errorText = matchingMsgs.reduce((prev, curr) => prev + `|${curr.id}[${curr.source}]`, ''); + errors.push({type: 'MSG', message: `Dup MSG: ${msg}, ${matchingMsgs.length} Locations: \n${errorText}`}); + } + } + return errors; +} + function collectMessageIds(docNode) { const messages = []; - const errors = []; if (docNode?.children?.length > 0) { for (const child of docNode.children) { const recursedResult = collectMessageIds(child); - messages.push(...recursedResult.messages); - errors.push(...recursedResult.errors); + messages.push(...recursedResult); } } const errorsFile = docNode?.['.errors'] if (errorsFile) { - fs.readFileSync(errorsFile, 'utf8').split('\n').forEach((line) => { + const lines = fs.readFileSync(errorsFile, 'utf8').split('\n') + for (const line of lines) { const shortErrorsPath = 'bin'+path.sep+errorsFile.split('bin'+path.sep)[1]; const pieces = line.trim().split('|'); if (pieces.length > 0 && pieces[0].trim().length > 0) { // check for duplicates // reconstruct full message string, in case it contained | characters const originalMsg = pieces.slice(2).join('|'); - const matchingMsgId = messages.find((item) => item.id === pieces[0] && item.message !== originalMsg); - const matchingMsgContent = messages.find((item) => item.message === pieces[2] && item.id !== pieces[0]); - if (matchingMsgId) { - errors.push({ type: 'ID', message: `Dup ID: |${pieces[0]}:${originalMsg}[${shortErrorsPath}]| VERSUS |${matchingMsgId.id}:${matchingMsgId.message}[${matchingMsgId.source}]|`}); - } - if (matchingMsgContent) { - errors.push({ type: 'MSG', message: `Dup MSG: |${pieces[0]}:${originalMsg}[${shortErrorsPath}]| VERSUS |${matchingMsgContent.id}:${matchingMsgContent.message}[${matchingMsgContent.source}]|`}) - } messages.push({ id: pieces[0], message: originalMsg, source: shortErrorsPath }); } - }) + } } - return { messages: messages, errors: errors}; + return messages; } diff --git a/.dependency/zwe_message_checks/package-lock.json b/.dependency/zwe_message_checks/package-lock.json index e35119fb72..e322a12543 100644 --- a/.dependency/zwe_message_checks/package-lock.json +++ b/.dependency/zwe_message_checks/package-lock.json @@ -10,6 +10,7 @@ "license": "EPL-2.0", "devDependencies": { "fs-extra": "^11.3.0", + "lodash": "^4.17.21", "string-comparison": "^1.3.0" } }, @@ -45,6 +46,12 @@ "graceful-fs": "^4.1.6" } }, + "node_modules/lodash": { + "version": "4.17.21", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", + "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", + "dev": true + }, "node_modules/string-comparison": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/string-comparison/-/string-comparison-1.3.0.tgz", diff --git a/.dependency/zwe_message_checks/package.json b/.dependency/zwe_message_checks/package.json index de0733ad40..2fbc4f7983 100644 --- a/.dependency/zwe_message_checks/package.json +++ b/.dependency/zwe_message_checks/package.json @@ -10,6 +10,7 @@ "license": "EPL-2.0", "devDependencies": { "fs-extra": "^11.3.0", + "lodash": "^4.17.21", "string-comparison": "^1.3.0" } } From bfed263100ee46ab740e8d8273556d4892c4c52b Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Wed, 5 Feb 2025 11:41:10 -0500 Subject: [PATCH 12/13] better output format Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.dependency/zwe_message_checks/index.js b/.dependency/zwe_message_checks/index.js index f12a4b70e9..0ccb58955b 100644 --- a/.dependency/zwe_message_checks/index.js +++ b/.dependency/zwe_message_checks/index.js @@ -96,7 +96,7 @@ function findDuplicates(collectedMsgs) { const matchingIds = _.uniqBy(collectedMsgs.filter((it) => it.id === id), 'message') //exclude the direct match if (matchingIds.length > 1) { - const errorText = matchingIds.reduce((prev, curr) => prev + `|${curr.message}[${curr.source}]|`, ''); + const errorText = matchingIds.reduce((prev, curr) => prev + `|${curr.message}[${curr.source}]|\n`, ''); errors.push({type: 'ID', message: `Dup ID: ${id}, ${matchingIds.length} Locations: \n${errorText}`}); } } @@ -104,7 +104,7 @@ function findDuplicates(collectedMsgs) { for (const msg of uniqMsgs) { const matchingMsgs = _.uniqBy(collectedMsgs.filter((it) => it.message === msg), 'id') if (matchingMsgs.length > 1) { - const errorText = matchingMsgs.reduce((prev, curr) => prev + `|${curr.id}[${curr.source}]`, ''); + const errorText = matchingMsgs.reduce((prev, curr) => prev + `|${curr.id}[${curr.source}]\n`, ''); errors.push({type: 'MSG', message: `Dup MSG: ${msg}, ${matchingMsgs.length} Locations: \n${errorText}`}); } } From 0c176d574adb1d309f6b3628fce45b4f47f6965b Mon Sep 17 00:00:00 2001 From: MarkAckert Date: Wed, 5 Feb 2025 11:51:52 -0500 Subject: [PATCH 13/13] small output tweaks Signed-off-by: MarkAckert --- .dependency/zwe_message_checks/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.dependency/zwe_message_checks/index.js b/.dependency/zwe_message_checks/index.js index 0ccb58955b..f4c3ad639a 100644 --- a/.dependency/zwe_message_checks/index.js +++ b/.dependency/zwe_message_checks/index.js @@ -47,7 +47,7 @@ console.log('---- Messages Used and Not Defined in .errors ----'); for(const msgSpec of discoveredMsgs) { for(const msg of msgSpec.messages) { if (!flatExpectedMessages.includes(msg.messageId)) { - console.log(`Missing message: ${JSON.stringify(msg)}`); + console.log(`|${msg.messageId}:${msg.message}[${msgSpec.src}]|\n`); statusFailed = true; continue; } @@ -104,7 +104,7 @@ function findDuplicates(collectedMsgs) { for (const msg of uniqMsgs) { const matchingMsgs = _.uniqBy(collectedMsgs.filter((it) => it.message === msg), 'id') if (matchingMsgs.length > 1) { - const errorText = matchingMsgs.reduce((prev, curr) => prev + `|${curr.id}[${curr.source}]\n`, ''); + const errorText = matchingMsgs.reduce((prev, curr) => prev + `|${curr.id}[${curr.source}]|\n`, ''); errors.push({type: 'MSG', message: `Dup MSG: ${msg}, ${matchingMsgs.length} Locations: \n${errorText}`}); } }