alex/esp-idf
alex/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2024-10-05 20:47:46 -04:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

Merge branch 'dangerjs/improove_jira_links_check' into 'master'

Browse Source 
DangerJS - Improved check for Jira links

Closes IDFSYNTEST-30, JIRA-123, IDF-6852, and IDF-6854

See merge request espressif/esp-idf!22457
This commit is contained in:
Tomas Sebestik 2023-03-08 17:25:29 +08:00
commit db39395a27
12 changed files with 428 additions and 200 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.gitlab/CODEOWNERS
View File

@ -50,7 +50,7 @@
/.github/workflows/ @esp-idf-codeowners/ci
/.gitlab-ci.yml @esp-idf-codeowners/ci
/.gitlab/ci/ @esp-idf-codeowners/ci
/.gitlab/dangerfile.js @esp-idf-codeowners/ci @esp-idf-codeowners/tools
/.gitlab/dangerjs/ @esp-idf-codeowners/ci @esp-idf-codeowners/tools
/.pre-commit-config.yaml @esp-idf-codeowners/ci
/.readthedocs.yml @esp-idf-codeowners/docs
/CMakeLists.txt @esp-idf-codeowners/build-config

10
.gitlab/ci/pre_check.yml
View File

@ -33,13 +33,13 @@ check_MR_style_dangerjs:
DANGER_GITLAB_API_TOKEN: ${ESPCI_TOKEN}
DANGER_GITLAB_HOST: ${GITLAB_HTTP_SERVER}
DANGER_GITLAB_API_BASE_URL: ${GITLAB_HTTP_SERVER}/api/v4
DANGER_JIRA_USER: ${DANGER_JIRA_USER}
DANGER_JIRA_PASSWORD: ${DANGER_JIRA_PASSWORD}
before_script:
- echo "Skip all before scripts"
- npm install -g danger@11.2.3 --silent --no-progress > /dev/null
- npm install axios@1.3.3 --silent --no-progress > /dev/null
script:
- set +e
- hash danger 2>/dev/null && echo "use cache" || yarn global add danger@11.2.3 --silent --skip-integrity-check --no-progress --cache-folder .yarn --global-folder .yarn-cache
- set -e
- danger ci --dangerfile=".gitlab/dangerfile.js" --failOnErrors -v
- danger ci --dangerfile=".gitlab/dangerjs/dangerfile.js" --failOnErrors -v
rules:
- if: '$CI_PIPELINE_SOURCE == "merge_request_event"'

194
.gitlab/dangerfile.js
View File

@ -1,194 +0,0 @@
import { danger, warn, message, results } from "danger"
/**
* Check if MR Title contains prefix "Draft: ... or "WIP: ...".
*
* @dangerjs WARN
*/
function checkMrTitle() {
const mrTitle = danger.gitlab.mr.title
const regexWip = /^WIP:/i;
const regexDraft = /^DRAFT:/i;
if ((regexWip.test(mrTitle)) || (regexDraft.test(mrTitle))) {
return warn("Please remove the `WIP:`/`DRAFT:` prefix from the MR name before merging this MR.");
}
}
checkMrTitle();
/**
* Check if MR Description is longer than 50 characters".
*
* @dangerjs WARN
*/
function checkMrDescription() {
const shortMrDescriptionThreshold = 50;// MR description is considered too short below this number of characters
const mrDescription = danger.gitlab.mr.description
if (mrDescription.length < shortMrDescriptionThreshold) {
return warn("The MR description looks very brief, please check if more details can be added.");
}
}
checkMrDescription();
/**
* Check if MR Description contains mandatory section "Release notes"
*
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6852
*
* @dangerjs WARN
*/
function checkMrReleaseNotes() {
const mrDescription = danger.gitlab.mr.description
if (!mrDescription.toUpperCase().includes("## Release notes".toUpperCase())) {
return warn("Please update the MR description, the mandatory section `Release Notes` seems to be missing.");
}
}
checkMrReleaseNotes();
/**
* Check if MR Description contains JIRA issues references
*
* Check if the associated GitHub Jira ticket has a GitHub closure reference in the commit message.
*
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6854
*
* @dangerjs WARN
*/
function checkMrJiraLinks() {
const mrDescription = danger.gitlab.mr.description
const mrCommitMessages = danger.gitlab.commits.map(commit => commit.message);
const matchBlockRelated = mrDescription.match(/\#\# Related.*$/s); // Match MR description starting with line ## Related till the end of MR description
const noRelatedIssues = /No related issues/.test(matchBlockRelated ? matchBlockRelated[0] : ''); // Check if there is "No related issues"
const testJiraLabels = /[A-Z]+-[0-9]+/.test(matchBlockRelated ? matchBlockRelated[0] : ''); // Test if pattern of Jira label "JIRA-1234" or "RDT-311" is in section Related
const ghIssueTicket = /IDFGH-[0-9]+/.test(matchBlockRelated ? matchBlockRelated[0] : ''); // Check if there is JIRA link starts with "IDFGH-*" in MR description, section "Related"
const testGithubLink = /Closes https:\/\/github\.com\/espressif\/esp-idf\/issues\/[0-9]+/
if (mrDescription.toUpperCase().includes("## RELATED") && noRelatedIssues) {
return
}
if (!mrDescription.toUpperCase().includes("## RELATED") || !testJiraLabels) { // Missing section "Related" or missing links to JIRA tickets
return message("Please consider adding references to JIRA issues in the `Related` section of the MR description.");
} else if (ghIssueTicket) { // Found JIRA ticket linked GitHub issue
if (!testGithubLink.test(mrCommitMessages)) { // Commit message does not contain a link to close the issue on GitHub
return warn("Please add GitHub issue closing link `Closes https://github.com/espressif/esp-idf/issues/<github-issue-number>` to the commit message.");
}
}
}
checkMrJiraLinks();
/**
* Check if MR has not an excessive numbers of commits (if squashed)
*
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6856.
*
* @dangerjs INFO
*/
function checkMrTooManyCommits() {
const tooManyCommitThreshold = 5 // above this number of commits, squash is recommended
const mrCommits = danger.gitlab.commits
if (mrCommits.length > tooManyCommitThreshold) {
return message(`You might consider squashing your ${mrCommits.length} commits (simplifying branch history).`);
}
}
checkMrTooManyCommits();
/**
* Check commit message are descriptive enough (longer that 10 characters)
* @dangerjs WARN
*/
function checkMrCommitMessagesLength() {
const shortCommitMessageThreshold = 10;// commit message is considered too short below this number of characters
const mrCommit = danger.gitlab.commits
let shortCommitMessages = [];
for (let i = 0; i < mrCommit.length; i++) {
const commitMessage = mrCommit[i].message;
if (commitMessage.length < shortCommitMessageThreshold) {
shortCommitMessages.push(`- commit message: ${commitMessage}`);
}
}
if (shortCommitMessages.length) {
warn(`Some of your commit messages may not be sufficiently descriptive (are shorter than ${shortCommitMessageThreshold} characters):
\n${shortCommitMessages.join("")}
\nYou might consider squashing commits (simplifying branch history) or updating those short commit messages.`);
}
}
checkMrCommitMessagesLength();
/**
* Check if MR is too large (more than 1000 lines of changes)
*
* @dangerjs INFO
*/
function checkMrIsTooLarge() {
const bigMrLinesOfCodeThreshold = 1000
danger.git.linesOfCode()
.then((totalLines) => {
if (totalLines > bigMrLinesOfCodeThreshold) {
return message(`This MR seems to be quiet large (total lines of code: ${totalLines}), you might consider splitting it into smaller MRs`);
}
});
}
checkMrIsTooLarge();
/**
* Check if documentation needs translation labels
*
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6855.
*
* @dangerjs WARN
*/
function checkMrNeedsTranlation() {
const mrLabels = danger.gitlab.mr.labels
const changesInDocsEN = /docs\/en/.test(danger.git.modified_files ? danger.git.modified_files[0] : ''); // Test if changes in directory "docs/EN"
const changesInDocsCH = /docs\/zh_CN/.test(danger.git.modified_files ? danger.git.modified_files[0] : ''); // Test if changes in directory "docs/CH"
// Only English docs has been changed
if (changesInDocsEN && !changesInDocsCH) {
if (!mrLabels.includes("needs translation: CN")) {
return warn("The updated documentation will need to be translated into Chinese, please add the MR label `needs translation: CN`");
}
}
// Only Chineese docs has been changed
if (!changesInDocsEN && changesInDocsCH) {
if (!mrLabels.includes("needs translation: EN")) {
return warn("The updated documentation will need to be translated into English, please add the MR label `needs translation: EN`");
}
}
}
checkMrNeedsTranlation();
/**
* Add a link to manual retry a DangerJS job (without committing to the repository)
*
* @dangerjs MARKDOWN
*/
function addRetryLink() {
const retryLink = `${process.env.DANGER_GITLAB_HOST}/${process.env.CI_PROJECT_PATH}/-/jobs/${process.env.CI_JOB_ID}`
return markdown(`***\n#### :repeat: If you want to run these checks again, please retry this [DangerJS job](${retryLink})\n***`);
}
addRetryLink();
function printSuccessLog() {
if (results.fails.length === 0 && results.warnings.length === 0 && results.messages.length === 0) {
return message('Good Job! All checks are passing!')
}
}
printSuccessLog();

42
.gitlab/dangerjs/dangerfile.js Normal file
View File

@ -0,0 +1,42 @@
/*
* Modules with checks are stored in ".gitlab/dangerjs/<module_name>". To import them, use path relative to "dangerfile.js"
*/
const checkMrCommits = require(`./mrCommitsCommitMessage.js`);
async function runChecks() {
// Checks for merge request title
require("./mrTitleNoDraftOrWip.js")();
// Checks for merge request description
require("./mrDescriptionLongEnough.js")();
require("./mrDescriptionHasReleaseNotes.js")();
await require('./mrDescriptionJiraLinks.js')();
// Checks for documentation
require("./mrDocsTranslation.js")();
// Checks for MR commits
require("./mrCommitsTooManyCommits.js")();
require("./mrCommitsCommitMessage.js")();
// Checks for MR code
require("./mrSizeTooLarge.js")();
// Add success log if no issues
if (
results.fails.length === 0 &&
results.warnings.length === 0 &&
results.messages.length === 0
) {
return message("Good Job! All checks are passing!");
}
}
runChecks();
// Add retry link
const retryLink = `${process.env.DANGER_GITLAB_HOST}/${process.env.CI_PROJECT_PATH}/-/jobs/${process.env.CI_JOB_ID}`;
markdown(
`***\n#### :repeat: You can enforce automatic MR checks by retrying the [DangerJS job](${retryLink})\n***`
);

26
.gitlab/dangerjs/mrCommitsCommitMessage.js Normal file
View File

@ -0,0 +1,26 @@
/**
* Check commit message are descriptive enough (longer that 10 characters)
*
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6856.
*
* @dangerjs WARN
*/
module.exports = function () {
const shortCommitMessageThreshold = 10; // commit message is considered too short below this number of characters
const mrCommit = danger.gitlab.commits;
let shortCommitMessages = [];
for (let i = 0; i < mrCommit.length; i++) {
const commitMessage = mrCommit[i].message;
if (commitMessage.length < shortCommitMessageThreshold) {
shortCommitMessages.push(`- commit message: ${commitMessage}`);
}
}
if (shortCommitMessages.length) {
warn(`Some of your commit messages may not be sufficiently descriptive (are shorter than ${shortCommitMessageThreshold} characters):
\n${shortCommitMessages.join("")}
\nYou might consider squashing commits (simplifying branch history) or updating those short commit messages.`);
}
};

17
.gitlab/dangerjs/mrCommitsTooManyCommits.js Normal file
View File

@ -0,0 +1,17 @@
/**
* Check if MR has not an excessive numbers of commits (if squashed)
*
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6856.
*
* @dangerjs INFO
*/
module.exports = function () {
const tooManyCommitThreshold = 5; // above this number of commits, squash is recommended
const mrCommits = danger.gitlab.commits;
if (mrCommits.length > tooManyCommitThreshold) {
return message(
`You might consider squashing your ${mrCommits.length} commits (simplifying branch history).`
);
}
};

28
.gitlab/dangerjs/mrDescriptionHasReleaseNotes.js Normal file
View File

@ -0,0 +1,28 @@
/**
* Check if MR Description contains mandatory section "Release notes"
*
* Extracts the content of the "Release notes" section from the GitLab merge request description.
*
* @dangerjs WARN (if section missing, is empty or wrong markdown format)
*/
module.exports = function () {
const mrDescription = danger.gitlab.mr.description;
const regexSectionReleaseNotes = /## Release notes([\s\S]*?)(?=## |$)/;
const sectionReleaseNotes = mrDescription.match(regexSectionReleaseNotes);
if (!sectionReleaseNotes) {
warn(
'The `Release Notes` section seems to be missing. Please check if the section header in MR description is present and in the correct markdown format ("## Release Notes")\n'
);
return null;
}
let content = sectionReleaseNotes[1].replace(/(\r\n|\n|\r)/gm, "").trim(); // Remove empty lines and whitespace
if (!content.length) {
warn(
"The `Release Notes` section seems to be empty (no section content)\n"
);
return null;
}
};

222
.gitlab/dangerjs/mrDescriptionJiraLinks.js Normal file
View File

@ -0,0 +1,222 @@
/** Check that there are valid JIRA links in MR desctiption.
*
* This check extracts the "Related" section from the MR description and
* searches for JIRA ticket references in the format "Closes [JIRA ticket key]".
*
* It then extracts the closing GitHub links from the corresponding JIRA tickets and
* checks if the linked GitHub issues are still in open state.
*
* Finally, it checks if the required GitHub closing links are present in the MR's commit messages.
*
*/
module.exports = async function () {
const axios = require("axios");
const mrDescription = danger.gitlab.mr.description;
const mrCommitMessages = danger.gitlab.commits.map(
(commit) => commit.message
);
let partMessages = []; // Create a blank field for future records of individual issues
// Parse section "Related" from MR Description
const sectionRelated = extractSectionRelated(mrDescription);
if (
!sectionRelated.header || // No section Related in MR description or ...
!/\s[A-Z]+-[0-9]+\s/.test(sectionRelated.content) // no Jira links in section Related
) {
return message(
"Please consider adding references to JIRA issues in the `Related` section of the MR description."
);
}
// Get closing (only) JIRA tickets
const jiraTickets = findClosingJiraTickets(sectionRelated.content);
for (const ticket of jiraTickets) {
ticket.jiraUIUrl = `https://jira.espressif.com:8443/browse/${ticket.ticketName}`;
if (!ticket.correctFormat) {
partMessages.push(
`- closing ticket \`${ticket.record}\` seems to be in the incorrect format. The correct format is for example \`- Closes JIRA-123\``
);
}
// Get closing GitHub issue links from JIRA tickets
const closingGithubLink = await getGitHubClosingLink(ticket.ticketName);
if (closingGithubLink) {
ticket.closingGithubLink = closingGithubLink;
} else if (closingGithubLink === null) {
partMessages.push(
`- the Jira issue number [\`${ticket.ticketName}\`](${ticket.jiraUIUrl}) seems to be invalid (please check if the ticket number is correct)`
);
continue; // Handle unreachable JIRA tickets; skip the following checks
} else {
continue; // Jira ticket have no GitHub closing link; skip the following checks
}
// Get still open GitHub issues
const githubIssueStatusOpen = await isGithubIssueOpen(
ticket.closingGithubLink
);
ticket.isOpen = githubIssueStatusOpen;
if (githubIssueStatusOpen === null) {
// Handle unreachable GitHub issues
partMessages.push(
`- the GitHub issue [\`${ticket.closingGithubLink}\`](${ticket.closingGithubLink}) does not seem to exist on GitHub (referenced from JIRA ticket [\`${ticket.ticketName}\`](${ticket.jiraUIUrl}) )`
);
continue; // skip the following checks
}
// Search in commit message if there are all GitHub closing links (from Related section) for still open GH issues
if (ticket.isOpen) {
if (
!mrCommitMessages.some((item) =>
item.includes(`Closes ${ticket.closingGithubLink}`)
)
) {
partMessages.push(
`- please add \`Closes ${ticket.closingGithubLink}\` to the commit message`
);
}
}
}
// Create report / DangerJS check feedback if issues with Jira links found
if (partMessages.length) {
createReport();
}
// ---------------------------------------------------------------
/**
* This function takes in a string mrDescription which contains a Markdown-formatted text
* related to a Merge Request (MR) in a GitLab repository. It searches for a section titled "Related"
* and extracts the content of that section. If the section is not found, it returns an object
* indicating that the header and content are null. If the section is found but empty, it returns
* an object indicating that the header is present but the content is null. If the section is found
* with content, it returns an object indicating that the header is present and the content of the
* "Related" section.
*
* @param {string} mrDescription - The Markdown-formatted text related to the Merge Request.
* @returns {{
* header: string | boolean | null,
* content: string | null
* }} - An object containing the header and content of the "Related" section, if present.
*/
function extractSectionRelated(mrDescription) {
const regexSectionRelated = /## Related([\s\S]*?)(?=## |$)/;
const sectionRelated = mrDescription.match(regexSectionRelated);
if (!sectionRelated) {
return { header: null, content: null }; // Section "Related" is missing
}
const content = sectionRelated[1].replace(/(\r\n|\n|\r)/gm, ""); // Remove empty lines
if (!content.length) {
return { header: true, content: null }; // Section "Related" is present, but empty
}
return { header: true, content: sectionRelated[1] }; // Found section "Related" with content
}
/**
* Finds all JIRA tickets that are being closed in the given sectionRelatedcontent.
* The function searches for lines that start with - Closes and have the format Closes [uppercase letters]-[numbers].
* @param {string} sectionRelatedcontent - A string that contains lines with mentions of JIRA tickets
* @returns {Array} An array of objects with ticketName property that has the correct format
*/
function findClosingJiraTickets(sectionRelatedcontent) {
let closingTickets = [];
const lines = sectionRelatedcontent.split("\n");
for (const line of lines) {
if (!line.startsWith("- Closes")) {
continue; // Not closing-type ticket, skip
}
const correctJiraClosingLinkFormat = /^- Closes [A-Z]+\-\d+$/;
if (!correctJiraClosingLinkFormat.test(line)) {
closingTickets.push({
record: line,
ticketName: line.match(/[A-Z]+\-\d+/)[0],
correctFormat: false,
});
} else {
closingTickets.push({
record: line,
ticketName: line.match(/[A-Z]+\-\d+/)[0],
correctFormat: true,
});
}
}
return closingTickets;
}
/**
* This function takes a JIRA issue key and retrieves the description from JIRA's API.
* It then searches the description for a GitHub closing link in the format "Closes https://github.com/owner/repo/issues/123".
* If a GitHub closing link is found, it is returned. If no GitHub closing link is found, it returns null.
* @param {string} jiraIssueKey - The key of the JIRA issue to search for the GitHub closing link.
* @returns {Promise<string|null>} - A promise that resolves to a string containing the GitHub closing link if found,
* or null if not found.
*/
async function getGitHubClosingLink(jiraIssueKey) {
let jiraDescrition = "";
// Get JIRA ticket description content
try {
const response = await axios({
url: `https://jira.espressif.com:8443/rest/api/latest/issue/${jiraIssueKey}`,
auth: {
username: process.env.DANGER_JIRA_USER,
password: process.env.DANGER_JIRA_PASSWORD,
},
});
jiraDescrition = response.data.fields.description;
} catch (error) {
return null;
}
// Find GitHub closing link in description
const regexClosingGhLink =
/Closes\s+(https:\/\/github.com\/\S+\/\S+\/issues\/\d+)/;
const closingGithubLink = jiraDescrition.match(regexClosingGhLink);
if (closingGithubLink) {
return closingGithubLink[1];
} else {
return false; // Jira issue has no GitHub closing link in description
}
}
/**
* Check if a GitHub issue linked in a merge request is still open.
*
* @param {string} link - The link to the GitHub issue.
* @returns {Promise<boolean>} A promise that resolves to a boolean indicating if the issue is open.
* @throws {Error} If the link is invalid or if there was an error fetching the issue.
*/
async function isGithubIssueOpen(link) {
const parsedUrl = new URL(link);
const [owner, repo] = parsedUrl.pathname.split("/").slice(1, 3);
const issueNumber = parsedUrl.pathname.split("/").slice(-1)[0];
try {
const response = await axios.get(
`https://api.github.com/repos/${owner}/${repo}/issues/${issueNumber}`
);
return response.data.state === "open"; // return True if GitHub issue is open
} catch (error) {
return null; // GET request to issue fails
}
}
function createReport() {
partMessages.sort();
let dangerMessage = `Some issues found for the related JIRA tickets in this MR:\n${partMessages.join(
"\n"
)}`;
warn(dangerMessage);
}
};

17
.gitlab/dangerjs/mrDescriptionLongEnough.js Normal file
View File

@ -0,0 +1,17 @@
/**
* Check if MR Description has accurate description".
*
* @dangerjs WARN
*/
module.exports = function () {
const mrDescription = danger.gitlab.mr.description;
const descriptionChunk = mrDescription.match(/^([^#]*)/)[1].trim(); // Extract all text before the first section header (i.e., the text before the "## Release notes")
const shortMrDescriptionThreshold = 50; // Description is considered too short below this number of characters
if (descriptionChunk.length < shortMrDescriptionThreshold) {
return warn(
"The MR description looks very brief, please check if more details can be added."
);
}
};

33
.gitlab/dangerjs/mrDocsTranslation.js Normal file
View File

@ -0,0 +1,33 @@
/**
* Check if documentation needs translation labels
*
* #TODO: this simple logic will be improved in future MRs - Jira IDF-6855.
*
* @dangerjs WARN
*/
module.exports = function () {
const mrLabels = danger.gitlab.mr.labels;
const changesInDocsEN = /docs\/en/.test(
danger.git.modified_files ? danger.git.modified_files[0] : ""
); // Test if changes in directory "docs/EN"
const changesInDocsCH = /docs\/zh_CN/.test(
danger.git.modified_files ? danger.git.modified_files[0] : ""
); // Test if changes in directory "docs/CH"
// Only English docs has been changed
if (changesInDocsEN && !changesInDocsCH) {
if (!mrLabels.includes("needs translation: CN")) {
return warn(
"The updated documentation will need to be translated into Chinese, please add the MR label `needs translation: CN`"
);
}
}
// Only Chineese docs has been changed
if (!changesInDocsEN && changesInDocsCH) {
if (!mrLabels.includes("needs translation: EN")) {
return warn(
"The updated documentation will need to be translated into English, please add the MR label `needs translation: EN`"
);
}
}
};

15
.gitlab/dangerjs/mrSizeTooLarge.js Normal file
View File

@ -0,0 +1,15 @@
/**
* Check if MR is too large (more than 1000 lines of changes)
*
* @dangerjs INFO
*/
module.exports = async function () {
const bigMrLinesOfCodeThreshold = 1000;
const totalLines = await danger.git.linesOfCode();
if (totalLines > bigMrLinesOfCodeThreshold) {
return message(
`This MR seems to be quite large (total lines of code: ${totalLines}), you might consider splitting it into smaller MRs`
);
}
};

22
.gitlab/dangerjs/mrTitleNoDraftOrWip.js Normal file
View File

@ -0,0 +1,22 @@
/**
* Check if MR Title contains prefix "WIP: ...".
*
* @dangerjs WARN
*/
module.exports = function () {
const mrTitle = danger.gitlab.mr.title;
const regexes = [
{ prefix: 'WIP', regex: /^WIP:/i },
{ prefix: 'W.I.P', regex: /^W\.I\.P/i },
{ prefix: '[WIP]', regex: /^\[WIP/i },
{ prefix: '[W.I.P]', regex: /^\[W\.I\.P/i },
{ prefix: '(WIP)', regex: /^\(WIP/i },
{ prefix: '(W.I.P)', regex: /^\(W\.I\.P/i },
];
for (const item of regexes) {
if (item.regex.test(mrTitle)) {
return warn(`Please remove the \`${item.prefix}\` prefix from the MR name before merging this MR.`);
}
}
};