Skip to content

Commit

Permalink
fix: make search queries of 256 characters maximum
Browse files Browse the repository at this point in the history
  • Loading branch information
pvdlg committed Feb 12, 2018
1 parent 73052e8 commit c556426
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 25 deletions.
12 changes: 12 additions & 0 deletions lib/get-search-queries.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module.exports = (base, commits, separator = '+') => {
return commits.reduce((searches, commit) => {
const lastSearch = searches[searches.length - 1];

if (lastSearch && lastSearch.length + commit.length <= 256 - separator.length) {
searches[searches.length - 1] = `${lastSearch}${separator}${commit}`;
} else {
searches.push(`${base}${separator}${commit}`);
}
return searches;
}, []);
};
13 changes: 9 additions & 4 deletions lib/success.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const issueParser = require('issue-parser')('github');
const debug = require('debug')('semantic-release:github');
const resolveConfig = require('./resolve-config');
const getClient = require('./get-client');
const getSearchQueries = require('./get-search-queries');
const getSuccessComment = require('./get-success-comment');
const findSRIssues = require('./find-sr-issues');

Expand All @@ -18,10 +19,14 @@ module.exports = async (
const github = getClient(githubToken, githubUrl, githubApiPathPrefix);
const releaseInfos = releases.filter(release => Boolean(release.name));

// Search for PRs associated with any commit in the release
const {data: {items: prs}} = await github.search.issues({
q: `${commits.map(commit => commit.hash).join('+')}+repo:${owner}/${repo}+type:pr`,
});
const prs = await pReduce(
getSearchQueries(`repo:${owner}/${repo}+type:pr`, commits.map(commit => commit.hash)),
async (prs, q) => {
const {data: {items}} = await github.search.issues({q});
return [...prs, ...items];
},
[]
);

debug('found pull requests: %O', prs.map(pr => pr.number));

Expand Down
34 changes: 34 additions & 0 deletions test/get-search-queries.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import test from 'ava';
import {repeat} from 'lodash';
import getSearchQueries from '../lib/get-search-queries';

test('Generate queries of 256 characters maximum', t => {
const commits = [
repeat('a', 40),
repeat('b', 40),
repeat('c', 40),
repeat('d', 40),
repeat('e', 40),
repeat('f', 40),
];

t.deepEqual(getSearchQueries(repeat('0', 51), commits), [
`${repeat('0', 51)}+${commits[0]}+${commits[1]}+${commits[2]}+${commits[3]}+${commits[4]}`,
`${repeat('0', 51)}+${commits[5]}`,
]);

t.deepEqual(getSearchQueries(repeat('0', 52), commits), [
`${repeat('0', 52)}+${commits[0]}+${commits[1]}+${commits[2]}+${commits[3]}`,
`${repeat('0', 52)}+${commits[4]}+${commits[5]}`,
]);
});

test('Generate one query if it is less tahn 256 characters', t => {
const commits = [repeat('a', 40), repeat('b', 40)];

t.deepEqual(getSearchQueries(repeat('0', 20), commits), [`${repeat('0', 20)}+${commits[0]}+${commits[1]}`]);
});

test('Return emty Array if there is no commits', t => {
t.deepEqual(getSearchQueries('base', []), []);
});
12 changes: 6 additions & 6 deletions test/integration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ test.serial('Comment on PR included in the releases', async t => {
.get(`/repos/${owner}/${repo}`)
.reply(200, {permissions: {push: true}})
.get(
`/search/issues?q=${commits.map(commit => commit.hash).join('+')}+${escape(`repo:${owner}/${repo}`)}+${escape(
'type:pr'
)}`
`/search/issues?q=${escape(`repo:${owner}/${repo}`)}+${escape('type:pr')}+${commits
.map(commit => commit.hash)
.join('+')}`
)
.reply(200, {items: prs})
.post(`/repos/${owner}/${repo}/issues/1/comments`, {body: /This PR is included/})
Expand Down Expand Up @@ -281,9 +281,9 @@ test.serial('Verify, release and notify success', async t => {
})
.reply(200, {upload_url: uploadUrl, html_url: releaseUrl})
.get(
`/search/issues?q=${commits.map(commit => commit.hash).join('+')}+${escape(`repo:${owner}/${repo}`)}+${escape(
'type:pr'
)}`
`/search/issues?q=${escape(`repo:${owner}/${repo}`)}+${escape('type:pr')}+${commits
.map(commit => commit.hash)
.join('+')}`
)
.reply(200, {items: prs})
.post(`/repos/${owner}/${repo}/issues/1/comments`, {body: /This PR is included/})
Expand Down
95 changes: 80 additions & 15 deletions test/success.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {escape} from 'querystring';
import test from 'ava';
import {repeat} from 'lodash';
import nock from 'nock';
import {stub} from 'sinon';
import ISSUE_ID from '../lib/definitions/sr-issue-id';
Expand Down Expand Up @@ -49,9 +50,9 @@ test.serial('Add comment to PRs associated with release commits and issues close
const releases = [{name: 'GitHub release', url: 'https://github.com/release'}];
const github = authenticate()
.get(
`/search/issues?q=${commits.map(commit => commit.hash).join('+')}+${escape(`repo:${owner}/${repo}`)}+${escape(
'type:pr'
)}`
`/search/issues?q=${escape(`repo:${owner}/${repo}`)}+${escape('type:pr')}+${commits
.map(commit => commit.hash)
.join('+')}`
)
.reply(200, {items: prs})
.post(`/repos/${owner}/${repo}/issues/1/comments`, {body: /This PR is included/})
Expand All @@ -78,6 +79,70 @@ test.serial('Add comment to PRs associated with release commits and issues close
t.true(github.isDone());
});

test.serial('Make multiple search queries if necessary', async t => {
const owner = 'test_user';
const repo = 'test_repo';
process.env.GITHUB_TOKEN = 'github_token';
const failTitle = 'The automated release is failing :rotating_light:';
const pluginConfig = {failTitle};
const prs = [
{number: 1, pull_request: {}},
{number: 2, pull_request: {}},
{number: 3, pull_request: {}},
{number: 4, pull_request: {}},
{number: 5, pull_request: {}},
{number: 6, pull_request: {}},
];
const options = {branch: 'master', repositoryUrl: `https://github.com/${owner}/${repo}.git`};
const commits = [
{hash: repeat('a', 40), message: 'Commit 1 message'},
{hash: repeat('b', 40), message: 'Commit 2 message'},
{hash: repeat('c', 40), message: 'Commit 3 message'},
{hash: repeat('d', 40), message: 'Commit 4 message'},
{hash: repeat('e', 40), message: 'Commit 5 message'},
{hash: repeat('f', 40), message: 'Commit 6 message'},
];
const nextRelease = {version: '1.0.0'};
const releases = [{name: 'GitHub release', url: 'https://github.com/release'}];
const github = authenticate()
.get(
`/search/issues?q=${escape(`repo:${owner}/${repo}`)}+${escape('type:pr')}+${commits[0].hash}+${commits[1].hash}+${
commits[2].hash
}+${commits[3].hash}+${commits[4].hash}`
)
.reply(200, {items: [prs[0], prs[1], prs[2], prs[3], prs[4]]})
.get(`/search/issues?q=${escape(`repo:${owner}/${repo}`)}+${escape('type:pr')}+${commits[5].hash}`)
.reply(200, {items: [prs[5]]})
.post(`/repos/${owner}/${repo}/issues/1/comments`, {body: /This PR is included/})
.reply(200, {html_url: 'https://github.com/successcomment-1'})
.post(`/repos/${owner}/${repo}/issues/2/comments`, {body: /This PR is included/})
.reply(200, {html_url: 'https://github.com/successcomment-2'})
.post(`/repos/${owner}/${repo}/issues/3/comments`, {body: /This PR is included/})
.reply(200, {html_url: 'https://github.com/successcomment-3'})
.post(`/repos/${owner}/${repo}/issues/4/comments`, {body: /This PR is included/})
.reply(200, {html_url: 'https://github.com/successcomment-4'})
.post(`/repos/${owner}/${repo}/issues/5/comments`, {body: /This PR is included/})
.reply(200, {html_url: 'https://github.com/successcomment-5'})
.post(`/repos/${owner}/${repo}/issues/6/comments`, {body: /This PR is included/})
.reply(200, {html_url: 'https://github.com/successcomment-6'})
.get(
`/search/issues?q=${escape(`title:${failTitle}`)}+${escape(`repo:${owner}/${repo}`)}+${escape(
'type:issue'
)}+${escape('state:open')}`
)
.reply(200, {items: []});

await success(pluginConfig, {options, commits, nextRelease, releases, logger: t.context.logger});

t.deepEqual(t.context.log.args[0], ['Added comment to issue #%d: %s', 1, 'https://github.com/successcomment-1']);
t.deepEqual(t.context.log.args[1], ['Added comment to issue #%d: %s', 2, 'https://github.com/successcomment-2']);
t.deepEqual(t.context.log.args[2], ['Added comment to issue #%d: %s', 3, 'https://github.com/successcomment-3']);
t.deepEqual(t.context.log.args[3], ['Added comment to issue #%d: %s', 4, 'https://github.com/successcomment-4']);
t.deepEqual(t.context.log.args[4], ['Added comment to issue #%d: %s', 5, 'https://github.com/successcomment-5']);
t.deepEqual(t.context.log.args[5], ['Added comment to issue #%d: %s', 6, 'https://github.com/successcomment-6']);
t.true(github.isDone());
});

test.serial('Do not add comment if no PR is associated with release commits', async t => {
const owner = 'test_user';
const repo = 'test_repo';
Expand All @@ -90,9 +155,9 @@ test.serial('Do not add comment if no PR is associated with release commits', as
const releases = [{name: 'GitHub release', url: 'https://github.com/release'}];
const github = authenticate()
.get(
`/search/issues?q=${commits.map(commit => commit.hash).join('+')}+${escape(`repo:${owner}/${repo}`)}+${escape(
'type:pr'
)}`
`/search/issues?q=${escape(`repo:${owner}/${repo}`)}+${escape('type:pr')}+${commits
.map(commit => commit.hash)
.join('+')}`
)
.reply(200, {items: []})
.get(
Expand Down Expand Up @@ -124,9 +189,9 @@ test.serial('Add custom comment', async t => {
const releases = [{name: 'GitHub release', url: 'https://github.com/release'}];
const github = authenticate()
.get(
`/search/issues?q=${commits.map(commit => commit.hash).join('+')}+${escape(`repo:${owner}/${repo}`)}+${escape(
'type:pr'
)}`
`/search/issues?q=${escape(`repo:${owner}/${repo}`)}+${escape('type:pr')}+${commits
.map(commit => commit.hash)
.join('+')}`
)
.reply(200, {items: prs})
.post(`/repos/${owner}/${repo}/issues/1/comments`, {
Expand Down Expand Up @@ -163,9 +228,9 @@ test.serial('Ignore errors when adding comments and closing issues', async t =>
const releases = [{name: 'GitHub release', url: 'https://github.com/release'}];
const github = authenticate()
.get(
`/search/issues?q=${commits.map(commit => commit.hash).join('+')}+${escape(`repo:${owner}/${repo}`)}+${escape(
'type:pr'
)}`
`/search/issues?q=${escape(`repo:${owner}/${repo}`)}+${escape('type:pr')}+${commits
.map(commit => commit.hash)
.join('+')}`
)
.reply(200, {items: prs})
.post(`/repos/${owner}/${repo}/issues/1/comments`, {body: /This PR is included/})
Expand Down Expand Up @@ -213,9 +278,9 @@ test.serial('Close open issues when a release is successful', async t => {
const releases = [{name: 'GitHub release', url: 'https://github.com/release'}];
const github = authenticate()
.get(
`/search/issues?q=${commits.map(commit => commit.hash).join('+')}+${escape(`repo:${owner}/${repo}`)}+${escape(
'type:pr'
)}`
`/search/issues?q=${escape(`repo:${owner}/${repo}`)}+${escape('type:pr')}+${commits
.map(commit => commit.hash)
.join('+')}`
)
.reply(200, {items: []})
.get(
Expand Down

0 comments on commit c556426

Please sign in to comment.