From 265ae650393f09e4c7d11b186fd9acdf234cd045 Mon Sep 17 00:00:00 2001 From: "Andrew C. Dvorak" Date: Wed, 25 Jul 2018 15:02:30 -0700 Subject: [PATCH 1/7] chore(infrastructure): Shorter PR comments (#3216) ### What it does - Shortens PR comments by removing unnecessary fluff - Adds some logging to help debug an error I've been seeing on Travis: - https://travis-ci.org/material-components/material-components-web/jobs/408248312 - https://travis-ci.org/material-components/material-components-web/jobs/408229973 ### Example output #### No diffs: ![image](https://user-images.githubusercontent.com/409245/43229795-dd23b910-901a-11e8-8025-3476ce5947ee.png) #### 64 diffs (collapsed): ![image](https://user-images.githubusercontent.com/409245/43229501-cd683dd0-9019-11e8-802c-0c51c6820d0f.png) #### 64 diffs (expanded): ![image](https://user-images.githubusercontent.com/409245/43229521-df7ba2aa-9019-11e8-806a-04b10eb40f88.png) --- test/screenshot/infra/commands/test.js | 45 ++++++++++++-------------- test/screenshot/infra/lib/git-repo.js | 11 +++++-- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/test/screenshot/infra/commands/test.js b/test/screenshot/infra/commands/test.js index 591ef012e7a..ef29166e22c 100644 --- a/test/screenshot/infra/commands/test.js +++ b/test/screenshot/infra/commands/test.js @@ -257,8 +257,21 @@ class TestCommand { * @private */ getPrComment_({masterDiffReportData, snapshotGitRev}) { - const reportPageUrl = masterDiffReportData.meta.report_html_file.public_url; + const masterReportPageUrl = masterDiffReportData.meta.report_html_file.public_url; const masterScreenshots = masterDiffReportData.screenshots; + const masterGitRev = masterDiffReportData.meta.golden_diff_base.git_revision; + + const numTotal = masterScreenshots.actual_screenshot_list.length; + const numChanged = + masterScreenshots.changed_screenshot_list.length + + masterScreenshots.added_screenshot_list.length + + masterScreenshots.removed_screenshot_list.length; + const plural = numChanged === 1 ? '' : 's'; + + if (numChanged === 0) { + const range = `commit ${snapshotGitRev.commit} vs. \`${masterGitRev.branch}\``; + return `**All ${numTotal} screenshot tests passed** for ${range}! 💯🎉`; + } const listMarkdown = [ this.getChangelistMarkdown_( @@ -272,38 +285,22 @@ class TestCommand { ), ].filter((str) => Boolean(str)).join('\n\n'); - let contentMarkdown; - const numChanged = - masterScreenshots.changed_screenshot_list.length + - masterScreenshots.added_screenshot_list.length + - masterScreenshots.removed_screenshot_list.length; + return ` +### Screenshot test report ⚠️ + +**${numChanged}** screenshot${plural} changed from \`${masterGitRev.branch}\` on commit ${snapshotGitRev.commit}: + +* ${masterReportPageUrl} - if (listMarkdown) { - contentMarkdown = `
- ${numChanged} screenshot${numChanged === 1 ? '' : 's'} changed ⚠️ + Details
${listMarkdown}
-`.trim(); - } else { - contentMarkdown = '### No diffs! 💯🎉'; - } - - return ` -🤖 Beep boop! - -### Screenshot test report - -Commit ${snapshotGitRev.commit} vs. \`master\`: - -* ${reportPageUrl} - -${contentMarkdown} `.trim(); } diff --git a/test/screenshot/infra/lib/git-repo.js b/test/screenshot/infra/lib/git-repo.js index da8a26d94d9..1ba239129df 100644 --- a/test/screenshot/infra/lib/git-repo.js +++ b/test/screenshot/infra/lib/git-repo.js @@ -164,7 +164,7 @@ class GitRepo { try { return this.repo_.checkIgnore(filePaths); } catch (err) { - throw new VError(err, `Failed to run GitRepo.getIgnoredPaths(${filePaths.length} file paths)`); + throw new VError(err, `Unable to check gitignore status of ${filePaths.length} file paths`); } } @@ -174,7 +174,14 @@ class GitRepo { */ async getCommitAuthor(commit = undefined) { /** @type {!Array} */ - const logEntries = await this.getLog([commit]); + let logEntries; + + try { + logEntries = await this.getLog([commit]); + } catch (err) { + throw new VError(err, `Unable to get author for commit "${commit}"`); + } + const logEntry = logEntries[0]; return User.create({ name: logEntry.author_name, From b613bd3ebfd7f7a1f607524393c87df474c50fd2 Mon Sep 17 00:00:00 2001 From: "Andrew C. Dvorak" Date: Wed, 25 Jul 2018 15:32:34 -0700 Subject: [PATCH 2/7] chore(infrastructure): Add stack traces for failing master merges (#3218) - https://travis-ci.org/material-components/material-components-web/jobs/408248312 - https://travis-ci.org/material-components/material-components-web/jobs/408229973 - https://travis-ci.org/material-components/material-components-web/jobs/408266376 --- test/screenshot/infra/lib/diff-base-parser.js | 17 ++++++++++------- test/screenshot/infra/lib/git-repo.js | 7 ++++--- test/screenshot/infra/lib/stacktrace.js | 6 ++++-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/test/screenshot/infra/lib/diff-base-parser.js b/test/screenshot/infra/lib/diff-base-parser.js index be9d97b9f90..9aaae052728 100644 --- a/test/screenshot/infra/lib/diff-base-parser.js +++ b/test/screenshot/infra/lib/diff-base-parser.js @@ -25,6 +25,7 @@ const {GOLDEN_JSON_RELATIVE_PATH} = require('./constants'); const Cli = require('./cli'); const GitHubApi = require('./github-api'); const GitRepo = require('./git-repo'); +const getStackTrace = require('./stacktrace')('DiffBaseParser'); const HTTP_URL_REGEX = new RegExp('^https?://'); @@ -201,9 +202,11 @@ class DiffBaseParser { const travisPrBranch = process.env.TRAVIS_PULL_REQUEST_BRANCH; const travisPrSha = process.env.TRAVIS_PULL_REQUEST_SHA; + const logInfo = {travisBranch, travisTag, travisPrNumber, travisPrBranch, travisPrSha}; + if (travisPrNumber) { const commit = await this.gitRepo_.getFullCommitHash(travisPrSha); - const author = await this.gitRepo_.getCommitAuthor(commit); + const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('getTravisGitRevision', logInfo)); return GitRevision.create({ type: GitRevision.Type.TRAVIS_PR, golden_json_file_path: GOLDEN_JSON_RELATIVE_PATH, @@ -217,7 +220,7 @@ class DiffBaseParser { if (travisTag) { const commit = await this.gitRepo_.getFullCommitHash(travisTag); - const author = await this.gitRepo_.getCommitAuthor(commit); + const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('getTravisGitRevision', logInfo)); return GitRevision.create({ type: GitRevision.Type.REMOTE_TAG, golden_json_file_path: GOLDEN_JSON_RELATIVE_PATH, @@ -229,7 +232,7 @@ class DiffBaseParser { if (travisBranch) { const commit = await this.gitRepo_.getFullCommitHash(travisBranch); - const author = await this.gitRepo_.getCommitAuthor(commit); + const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('getTravisGitRevision', logInfo)); return GitRevision.create({ type: GitRevision.Type.LOCAL_BRANCH, golden_json_file_path: GOLDEN_JSON_RELATIVE_PATH, @@ -298,7 +301,7 @@ class DiffBaseParser { * @private */ async createCommitDiffBase_(commit, goldenJsonFilePath) { - const author = await this.gitRepo_.getCommitAuthor(commit); + const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('createCommitDiffBase_')); return DiffBase.create({ type: DiffBase.Type.GIT_REVISION, @@ -323,7 +326,7 @@ class DiffBaseParser { const remote = allRemoteNames.find((curRemoteName) => remoteRef.startsWith(curRemoteName + '/')); const branch = remoteRef.substr(remote.length + 1); // add 1 for forward-slash separator const commit = await this.gitRepo_.getFullCommitHash(remoteRef); - const author = await this.gitRepo_.getCommitAuthor(commit); + const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('createRemoteBranchDiffBase_')); return DiffBase.create({ type: DiffBase.Type.GIT_REVISION, @@ -347,7 +350,7 @@ class DiffBaseParser { */ async createRemoteTagDiffBase_(tagRef, goldenJsonFilePath) { const commit = await this.gitRepo_.getFullCommitHash(tagRef); - const author = await this.gitRepo_.getCommitAuthor(commit); + const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('createRemoteTagDiffBase_')); return DiffBase.create({ type: DiffBase.Type.GIT_REVISION, @@ -371,7 +374,7 @@ class DiffBaseParser { */ async createLocalBranchDiffBase_(branch, goldenJsonFilePath) { const commit = await this.gitRepo_.getFullCommitHash(branch); - const author = await this.gitRepo_.getCommitAuthor(commit); + const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('createLocalBranchDiffBase_')); return DiffBase.create({ type: DiffBase.Type.GIT_REVISION, diff --git a/test/screenshot/infra/lib/git-repo.js b/test/screenshot/infra/lib/git-repo.js index 1ba239129df..07c9e3eb476 100644 --- a/test/screenshot/infra/lib/git-repo.js +++ b/test/screenshot/infra/lib/git-repo.js @@ -169,17 +169,18 @@ class GitRepo { } /** - * @param {string=} commit + * @param {string} commit + * @param {string} stackTrace * @return {!Promise} */ - async getCommitAuthor(commit = undefined) { + async getCommitAuthor(commit, stackTrace) { /** @type {!Array} */ let logEntries; try { logEntries = await this.getLog([commit]); } catch (err) { - throw new VError(err, `Unable to get author for commit "${commit}"`); + throw new VError(err, `Unable to get author for commit "${commit}":\n${stackTrace}`); } const logEntry = logEntries[0]; diff --git a/test/screenshot/infra/lib/stacktrace.js b/test/screenshot/infra/lib/stacktrace.js index fbfe5741d1b..3c7f4b026d5 100644 --- a/test/screenshot/infra/lib/stacktrace.js +++ b/test/screenshot/infra/lib/stacktrace.js @@ -19,17 +19,19 @@ const colors = require('colors'); /** * @param {string} className + * @param {*=} infoData * @return {function(methodName: string): string} */ -module.exports = function(className) { +module.exports = function(className, infoData = undefined) { /** * @param {string} methodName * @return {string} */ function getStackTrace(methodName) { + const infoStr = typeof infoData === 'object' ? '\n' + JSON.stringify(infoData, null, 2) : ''; const fullStack = new Error(`${className}.${methodName}()`).stack; // Remove THIS function from the stack trace because it's not useful - return fullStack.split('\n').filter((line, index) => index !== 1).join('\n'); + return fullStack.split('\n').filter((line, index) => index !== 1).join('\n') + infoStr; } return getStackTrace; From 36e9516041a2589b800c5fadc271964a38bc1e1b Mon Sep 17 00:00:00 2001 From: Will Ernest <34519388+williamernest@users.noreply.github.com> Date: Wed, 25 Jul 2018 15:55:40 -0700 Subject: [PATCH 3/7] chore(infrastructure): Add chrome to tests for local debugging (#3219) --- karma.conf.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/karma.conf.js b/karma.conf.js index 380d7dca3cd..4fddec16d85 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -134,7 +134,14 @@ const SAUCE_LAUNCHERS = { // }, }; -const getLaunchers = () => USING_SL ? SAUCE_LAUNCHERS : LOCAL_LAUNCHERS; +const getLaunchers = () => { + if (USING_TRAVISCI) { + return USING_SL ? SAUCE_LAUNCHERS : LOCAL_LAUNCHERS; + } else { + return ['Chrome']; + } +}; + const getBrowsers = () => Object.keys(getLaunchers()); module.exports = function(config) { From e513ddbf4b2385fb6fda780f2bf34bec31e022a4 Mon Sep 17 00:00:00 2001 From: Will Ernest <34519388+williamernest@users.noreply.github.com> Date: Wed, 25 Jul 2018 16:30:30 -0700 Subject: [PATCH 4/7] chore(infrastructure): Fix local tests (#3223) --- karma.conf.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/karma.conf.js b/karma.conf.js index 4fddec16d85..4a8cfc70d37 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -134,15 +134,8 @@ const SAUCE_LAUNCHERS = { // }, }; -const getLaunchers = () => { - if (USING_TRAVISCI) { - return USING_SL ? SAUCE_LAUNCHERS : LOCAL_LAUNCHERS; - } else { - return ['Chrome']; - } -}; - -const getBrowsers = () => Object.keys(getLaunchers()); +const getLaunchers = () => USING_SL ? SAUCE_LAUNCHERS : LOCAL_LAUNCHERS; +const getBrowsers = () => USING_TRAVISCI ? Object.keys(getLaunchers()) : ['Chrome']; module.exports = function(config) { config.set({ From e6ec57cb66066cde059a40da1f4b96001e16eb71 Mon Sep 17 00:00:00 2001 From: "Andrew C. Dvorak" Date: Wed, 25 Jul 2018 17:35:40 -0700 Subject: [PATCH 5/7] chore(infrastructure): Fix Travis tests on feature branch PRs (#3226) - Fetches `$TRAVIS_BRANCH` and `$TRAVIS_PULL_REQUEST_BRANCH` git branches to ensure they are available for subsequent `git log` queries --- test/screenshot/infra/commands/travis.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/screenshot/infra/commands/travis.sh b/test/screenshot/infra/commands/travis.sh index a91db094439..079a02e06c3 100755 --- a/test/screenshot/infra/commands/travis.sh +++ b/test/screenshot/infra/commands/travis.sh @@ -18,6 +18,19 @@ function print_travis_env_vars() { echo } +function maybe_add_git_branch() { + if [[ -n "$1" ]]; then + git remote set-branches --add origin "$1" + fi +} + +function maybe_fetch_git_branches() { + maybe_add_git_branch 'master' + maybe_add_git_branch "$TRAVIS_BRANCH" + maybe_add_git_branch "$TRAVIS_PULL_REQUEST_BRANCH" + git fetch --tags +} + function maybe_extract_api_credentials() { if [[ -z "$encrypted_eead2343bb54_key" ]] || [[ -z "$encrypted_eead2343bb54_iv" ]]; then log_error @@ -83,6 +96,7 @@ function maybe_install_gcloud_sdk() { if [[ "$TEST_SUITE" == 'screenshot' ]]; then print_travis_env_vars + maybe_fetch_git_branches maybe_extract_api_credentials maybe_install_gcloud_sdk fi From ce4d9d0b39cef7606986e633f7f9e0898a1b5312 Mon Sep 17 00:00:00 2001 From: "Andrew C. Dvorak" Date: Wed, 25 Jul 2018 21:37:27 -0700 Subject: [PATCH 6/7] chore(infrastructure): Throw more descriptive errors in GitRepo class (#3228) Also: - Fixes typo in `getStackTrace()` --- test/screenshot/infra/commands/travis.sh | 1 + test/screenshot/infra/lib/git-repo.js | 18 +++++++++++++++--- test/screenshot/infra/lib/stacktrace.js | 6 +++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/test/screenshot/infra/commands/travis.sh b/test/screenshot/infra/commands/travis.sh index 079a02e06c3..df43215681a 100755 --- a/test/screenshot/infra/commands/travis.sh +++ b/test/screenshot/infra/commands/travis.sh @@ -20,6 +20,7 @@ function print_travis_env_vars() { function maybe_add_git_branch() { if [[ -n "$1" ]]; then + # https://github.com/marionebl/commitlint/issues/6#issuecomment-231186598 git remote set-branches --add origin "$1" fi } diff --git a/test/screenshot/infra/lib/git-repo.js b/test/screenshot/infra/lib/git-repo.js index 07c9e3eb476..4d77bd31e07 100644 --- a/test/screenshot/infra/lib/git-repo.js +++ b/test/screenshot/infra/lib/git-repo.js @@ -79,7 +79,11 @@ class GitRepo { * @return {!Promise} */ async getFullCommitHash(ref = 'HEAD') { - return this.exec_('revparse', [ref]); + const hash = this.exec_('revparse', [ref]); + if (!hash) { + throw new Error(`Unable to get commit hash for git ref "${ref}"`); + } + return hash; } /** @@ -87,7 +91,11 @@ class GitRepo { * @return {!Promise} */ async getBranchName(ref = 'HEAD') { - return this.exec_('revparse', ['--abbrev-ref', ref]); + const branch = this.exec_('revparse', ['--abbrev-ref', ref]); + if (!branch) { + throw new Error(`Unable to get branch name for git ref "${ref}"`); + } + return branch; } /** @@ -102,7 +110,11 @@ class GitRepo { * @return {!Promise} */ async getFullSymbolicName(ref = 'HEAD') { - return this.exec_('revparse', ['--symbolic-full-name', ref]); + const fullName = this.exec_('revparse', ['--symbolic-full-name', ref]); + if (!fullName) { + throw new Error(`Unable to get full symbolic name for git ref "${ref}"`); + } + return fullName; } /** diff --git a/test/screenshot/infra/lib/stacktrace.js b/test/screenshot/infra/lib/stacktrace.js index 3c7f4b026d5..01d5b454220 100644 --- a/test/screenshot/infra/lib/stacktrace.js +++ b/test/screenshot/infra/lib/stacktrace.js @@ -19,15 +19,15 @@ const colors = require('colors'); /** * @param {string} className - * @param {*=} infoData * @return {function(methodName: string): string} */ -module.exports = function(className, infoData = undefined) { +module.exports = function(className) { /** * @param {string} methodName + * @param {*=} infoData * @return {string} */ - function getStackTrace(methodName) { + function getStackTrace(methodName, infoData = undefined) { const infoStr = typeof infoData === 'object' ? '\n' + JSON.stringify(infoData, null, 2) : ''; const fullStack = new Error(`${className}.${methodName}()`).stack; // Remove THIS function from the stack trace because it's not useful From ffb8bb70d34ed35d2223e332a720a8237dc5fbc5 Mon Sep 17 00:00:00 2001 From: "Andrew C. Dvorak" Date: Thu, 26 Jul 2018 08:33:02 -0700 Subject: [PATCH 7/7] chore(infrastructure): Try to fix Travis for merges into master (#3232) ### What it does - (Hopefully) Fixes failing screenshot tests on Travis for merges into `master` - Uses the `TRAVIS_COMMIT` env var instead of querying `git` - Looking up the commit with `git rev-parse master` seems to fail on Travis for some reason - Stops trying to set GitHub commit status if API credentials are not found - Previously, this would throw an error and kill the whole test - Adds a bit of logging --- test/screenshot/infra/lib/diff-base-parser.js | 27 ++++++++--------- test/screenshot/infra/lib/git-repo.js | 4 +++ test/screenshot/infra/lib/github-api.js | 29 ++++++++++++++++--- 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/test/screenshot/infra/lib/diff-base-parser.js b/test/screenshot/infra/lib/diff-base-parser.js index 9aaae052728..bbc1f1d99db 100644 --- a/test/screenshot/infra/lib/diff-base-parser.js +++ b/test/screenshot/infra/lib/diff-base-parser.js @@ -114,14 +114,14 @@ class DiffBaseParser { // E.g.: `--diff-base=https://storage.googleapis.com/.../golden.json` const isUrl = HTTP_URL_REGEX.test(rawDiffBase); if (isUrl) { - return this.createPublicUrlDiffBase_(rawDiffBase); + return await this.createPublicUrlDiffBase_(rawDiffBase); } // Diff against a local `golden.json` file. // E.g.: `--diff-base=/tmp/golden.json` const isLocalFile = await fs.exists(rawDiffBase); if (isLocalFile) { - return this.createLocalFileDiffBase_(rawDiffBase); + return await this.createLocalFileDiffBase_(rawDiffBase); } const [inputGoldenRef, inputGoldenPath] = rawDiffBase.split(':'); @@ -141,7 +141,7 @@ class DiffBaseParser { // Diff against a specific git commit. // E.g.: `--diff-base=abcd1234` if (!fullGoldenRef) { - return this.createCommitDiffBase_(inputGoldenRef, goldenFilePath); + return await this.createCommitDiffBase_(inputGoldenRef, goldenFilePath); } const {remoteRef, localRef, tagRef} = this.getRefType_(fullGoldenRef); @@ -149,18 +149,18 @@ class DiffBaseParser { // Diff against a remote git branch. // E.g.: `--diff-base=origin/master` or `--diff-base=origin/feat/button/my-fancy-feature` if (remoteRef) { - return this.createRemoteBranchDiffBase_(remoteRef, goldenFilePath); + return await this.createRemoteBranchDiffBase_(remoteRef, goldenFilePath); } // Diff against a remote git tag. // E.g.: `--diff-base=v0.34.1` if (tagRef) { - return this.createRemoteTagDiffBase_(tagRef, goldenFilePath); + return await this.createRemoteTagDiffBase_(tagRef, goldenFilePath); } // Diff against a local git branch. // E.g.: `--diff-base=master` or `--diff-base=HEAD` - return this.createLocalBranchDiffBase_(localRef, goldenFilePath); + return await this.createLocalBranchDiffBase_(localRef, goldenFilePath); } /** @@ -201,12 +201,17 @@ class DiffBaseParser { const travisPrNumber = Number(process.env.TRAVIS_PULL_REQUEST); const travisPrBranch = process.env.TRAVIS_PULL_REQUEST_BRANCH; const travisPrSha = process.env.TRAVIS_PULL_REQUEST_SHA; + const travisCommit = process.env.TRAVIS_COMMIT; + const commit = travisPrSha || travisCommit; - const logInfo = {travisBranch, travisTag, travisPrNumber, travisPrBranch, travisPrSha}; + if (!commit) { + return null; + } + + const logInfo = {travisBranch, travisTag, travisPrNumber, travisPrBranch, travisPrSha, travisCommit}; + const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('getTravisGitRevision', logInfo)); if (travisPrNumber) { - const commit = await this.gitRepo_.getFullCommitHash(travisPrSha); - const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('getTravisGitRevision', logInfo)); return GitRevision.create({ type: GitRevision.Type.TRAVIS_PR, golden_json_file_path: GOLDEN_JSON_RELATIVE_PATH, @@ -219,8 +224,6 @@ class DiffBaseParser { } if (travisTag) { - const commit = await this.gitRepo_.getFullCommitHash(travisTag); - const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('getTravisGitRevision', logInfo)); return GitRevision.create({ type: GitRevision.Type.REMOTE_TAG, golden_json_file_path: GOLDEN_JSON_RELATIVE_PATH, @@ -231,8 +234,6 @@ class DiffBaseParser { } if (travisBranch) { - const commit = await this.gitRepo_.getFullCommitHash(travisBranch); - const author = await this.gitRepo_.getCommitAuthor(commit, getStackTrace('getTravisGitRevision', logInfo)); return GitRevision.create({ type: GitRevision.Type.LOCAL_BRANCH, golden_json_file_path: GOLDEN_JSON_RELATIVE_PATH, diff --git a/test/screenshot/infra/lib/git-repo.js b/test/screenshot/infra/lib/git-repo.js index 4d77bd31e07..593e3c837c2 100644 --- a/test/screenshot/infra/lib/git-repo.js +++ b/test/screenshot/infra/lib/git-repo.js @@ -196,6 +196,10 @@ class GitRepo { } const logEntry = logEntries[0]; + if (!logEntry) { + throw new VError(err, `Unable to get author for commit "${commit}":\n${stackTrace}`); + } + return User.create({ name: logEntry.author_name, email: logEntry.author_email, diff --git a/test/screenshot/infra/lib/github-api.js b/test/screenshot/infra/lib/github-api.js index 0a9d490a61c..fba2ae9f6f1 100644 --- a/test/screenshot/infra/lib/github-api.js +++ b/test/screenshot/infra/lib/github-api.js @@ -18,6 +18,9 @@ const VError = require('verror'); const debounce = require('debounce'); const octokit = require('@octokit/rest'); +/** @type {!CliColor} */ +const colors = require('colors'); + const GitRepo = require('./git-repo'); const getStackTrace = require('./stacktrace')('GitHubApi'); @@ -25,12 +28,13 @@ class GitHubApi { constructor() { this.gitRepo_ = new GitRepo(); this.octokit_ = octokit(); + this.isAuthenticated_ = false; + this.hasWarnedNoAuth_ = false; this.authenticate_(); + this.initStatusThrottle_(); } - /** - * @private - */ + /** @private */ authenticate_() { let token; @@ -46,6 +50,11 @@ class GitHubApi { token: token, }); + this.isAuthenticated_ = true; + } + + /** @private */ + initStatusThrottle_() { const throttle = (fn, delay) => { let lastCall = 0; return (...args) => { @@ -163,7 +172,19 @@ class GitHubApi { * @private */ async createStatusUnthrottled_({state, targetUrl, description = undefined}) { - const sha = process.env.TRAVIS_PULL_REQUEST_SHA || await this.gitRepo_.getFullCommitHash(); + if (!this.isAuthenticated_) { + if (!this.hasWarnedNoAuth_) { + const warning = colors.magenta('WARNING'); + console.warn(`${warning}: Cannot set GitHub commit status because no API credentials were found.`); + this.hasWarnedNoAuth_ = true; + } + return null; + } + + const travisPrSha = process.env.TRAVIS_PULL_REQUEST_SHA; + const travisCommit = process.env.TRAVIS_COMMIT; + const sha = travisPrSha || travisCommit || await this.gitRepo_.getFullCommitHash(); + let stackTrace; try {