From 7bb88f47210d4e3c78a631742dfedbdc34215a3a Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Sun, 6 Nov 2022 14:53:41 +0000 Subject: [PATCH] Initial report for missing lines. --- .github/scripts/pr_comment.js | 109 ++++++++++++++++---- .github/scripts/pr_comment.test.body.txt | 1 + .github/scripts/pr_comment.test.js | 72 +++++++++++++ .github/scripts/pr_comment.test.report.json | 1 + .github/workflows/ci.yml | 8 +- src/towncrier/newsfragments/450.misc | 0 6 files changed, 167 insertions(+), 24 deletions(-) create mode 100644 .github/scripts/pr_comment.test.body.txt create mode 100644 .github/scripts/pr_comment.test.js create mode 100644 .github/scripts/pr_comment.test.report.json create mode 100644 src/towncrier/newsfragments/450.misc diff --git a/.github/scripts/pr_comment.js b/.github/scripts/pr_comment.js index c6b94ecf..afc5aed4 100644 --- a/.github/scripts/pr_comment.js +++ b/.github/scripts/pr_comment.js @@ -7,42 +7,35 @@ Update the content of the existing comment. https://octokit.github.io/rest.js/v19 */ -module.exports = async ({github, context, process}) => { - var octokit_rest = github +module.exports = async ({github, context, process, retry_delay}) => { + // Create the namespace to make it easy to copy/paste example from the + // octokit docs. + const octokit = {rest: github} + + const comment_marker = '\n' + process.env.COMMENT_MARKER + if (context.eventName != "pull_request") { // Only PR are supported. return } - var sleep = function(second) { + var sleep = (second) => { return new Promise(resolve => setTimeout(resolve, second * 1000)) } /* - Perform the actual logic. - - This is wrapped so that we can retry on errors. + Create or update the PR summary report as a single comment. */ - var doAction = async function() { - - console.log(context) - - fs = require('fs'); - - const body = fs.readFileSync( - process.env.GITHUB_WORKSPACE + "/" + process.env.COMMENT_BODY, 'utf8'); + var doSummaryComment = async (body) => { var comment_id = null - var comment_marker = '\n' + process.env.COMMENT_MARKER - var comment_body = body + comment_marker + const comment_body = body + comment_marker - var comments = await octokit_rest.issues.listComments({ + const comments = await octokit.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.payload.number, }) - console.log(comments) - comments.data.forEach(comment => { if (comment.body.endsWith(comment_marker)) { comment_id = comment.id @@ -52,7 +45,7 @@ module.exports = async ({github, context, process}) => { if (comment_id) { // We have an existing comment. // update the content. - await octokit_rest.issues.updateComment({ + await octokit.rest.issues.updateComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: comment_id, @@ -62,7 +55,7 @@ module.exports = async ({github, context, process}) => { } // Create a new comment. - await octokit_rest.issues.createComment({ + await octokit.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: context.payload.number, @@ -71,10 +64,82 @@ module.exports = async ({github, context, process}) => { } + /* + Create or refresh the diff inline comments. + */ + var doDiffComments = async (report) => { + + var review_id = null + const existing_reviews = await octokit.rest.pulls.listReviews({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.number, + }) + existing_reviews.data.forEach((review) => { + if (review.body.endsWith(comment_marker)) { + review_id = review.id + } + }) + + if (review_id) { + // We have an existing review. + // Delete it as we will replace it with a new one. + + } + + // Prepare inline comments. + var comments = [] + Object.keys(report.src_stats).forEach((path) => { + report.src_stats[path].violation_lines.forEach((position) => { + comments.push({ + path, + position, + body: 'Missing coverage.' + }) + }) + }) + + if (!comments) { + // Coverage is complete. Nothing to comment about. + return + } + + await octokit.rest.pulls.createReview({ + owner: context.repo.owner, + repo: context.repo.repo, + commit_id: context.payload.after, + pull_number: context.payload.number, + event: "COMMENT", + body: "Missing coverage report." + comment_marker, + comments + }) + } + + /* + Perform the actual logic. + + This is wrapped so that we can retry on errors. + */ + var doAction = async () => { + + console.log(context) + + fs = require('fs'); + + const body = fs.readFileSync( + process.env.GITHUB_WORKSPACE + "/" + process.env.COMMENT_BODY, 'utf8') + await doSummaryComment(body) + + const report_json = fs.readFileSync( + process.env.GITHUB_WORKSPACE + "/" + process.env.REPORT_JSON, 'utf8'); + await doDiffComments(JSON.parse(report_json)) + + } + try { await doAction() } catch (e) { - await sleep(5) + await sleep(retry_delay) await doAction() } } diff --git a/.github/scripts/pr_comment.test.body.txt b/.github/scripts/pr_comment.test.body.txt new file mode 100644 index 00000000..f7fd7efd --- /dev/null +++ b/.github/scripts/pr_comment.test.body.txt @@ -0,0 +1 @@ +Dummy body of the summary PR comment. diff --git a/.github/scripts/pr_comment.test.js b/.github/scripts/pr_comment.test.js new file mode 100644 index 00000000..e20a161e --- /dev/null +++ b/.github/scripts/pr_comment.test.js @@ -0,0 +1,72 @@ +/* +This should be called via nodejs from the repo root folder. + +node .github/scripts/pr_comment.test.js + +This is just a poor man's testing code for the script. +Since the project is mostly python, is not easy to add nodeenv and a full +testing suite like jest. +*/ +const process = require('process'); + +process.env.GITHUB_WORKSPACE = process.cwd() +process.env.COMMENT_BODY = '.github/scripts/pr_comment.test.body.txt' +process.env.REPORT_JSON = '.github/scripts/pr_comment.test.report.json' +process.env.COMMENT_MARKER = '' + +const script = require(process.cwd() + '/.github/scripts/pr_comment.js') +const retry_delay = 0.1 + +var github = { + issues: { + listComments: (options) => { + console.log('Retrieving comments...') + console.log(options) + return {data: [ + {'id': 980, 'body': 'some-other-comment'}, + // Comment the line below to trigger creating a new comment. + {'id': 456, 'body': 'comment marker\n'}, + {'body': 'another-other-comment'}, + + ]} + }, + updateComment: (options) => { + console.log('Updating existing comment...') + console.log(options) + }, + createComment: (options) => { + console.log('Creating new comment...') + console.log(options) + }, + }, + pulls: { + listReviews: (options) => { + console.log('Listing existing revies...') + console.log(options) + return {data: [ + {'id': 120, 'body': 'some-other-pr-review'}, + // Comment the line below to trigger creating a new comment. + {'id': 156, 'body': 'review marker\n'}, + ]} + }, + createReview: (options) => { + console.log('Creating new review...') + console.log(options) + }, + }, +} + +var context = { + eventName: 'pull_request', + repo: { + owner: 'dummy-org', + repo: 'dummy-repo', + }, + payload: { + number: 123, + after: '123abc567', + } +} + + +script({github, context, process, retry_delay}) diff --git a/.github/scripts/pr_comment.test.report.json b/.github/scripts/pr_comment.test.report.json new file mode 100644 index 00000000..ec400797 --- /dev/null +++ b/.github/scripts/pr_comment.test.report.json @@ -0,0 +1 @@ +{"report_name": "File generate by diff-cover --json-report out.json", "diff_name": "trunk...HEAD, staged and unstaged changes", "src_stats": {"src/towncrier/build.py": {"percent_covered": 22.222222222222214, "violation_lines": [123, 125, 126, 138, 140, 141, 142], "violations": [[123, null], [125, null], [126, null], [138, null], [140, null], [141, null], [142, null]]}}, "total_num_lines": 9, "total_num_violations": 7, "total_percent_covered": 22, "num_changed_lines": 185} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3757d50b..866dafa1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -262,6 +262,8 @@ jobs: # Even we send a comment to a PR, we use the issues API. # Issues and PR share the same comment API. issues: write + # This is for sending inline review comments. + pull-requests: write needs: # We are waiting only for test jobs. - test-linux @@ -293,7 +295,7 @@ jobs: echo '```' > coverage-report.txt coverage report --show-missing >> coverage-report.txt echo '```' >> coverage-report.txt - diff-cover --markdown-report diff-cover.md --compare-branch origin/trunk coverage.xml + diff-cover --markdown-report diff-cover.md --json-report diff-cover.json --compare-branch origin/trunk coverage.xml cat diff-cover.md >> coverage-report.txt # Use the generic JS script to call our custom script @@ -303,11 +305,13 @@ jobs: env: COMMENT_MARKER: "" COMMENT_BODY: coverage-report.txt + REPORT_JSON: diff-cover.json with: script: | const script = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/pr_comment.js`) // Only pass top level objects as GHA does dependecy injection. - await script({github, context, process}) + const retry_delay = 5 + await script({github, context, process, retry_delay}) - name: Enforce diff coverage run: | diff --git a/src/towncrier/newsfragments/450.misc b/src/towncrier/newsfragments/450.misc new file mode 100644 index 00000000..e69de29b