Skip to content

Add cutoff repos and counts to error message #1408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
robertbrignull merged 1 commit into from
Jun 28, 2022
+82 −11
Merged

Add cutoff repos and counts to error message #1408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions extensions/ql-vscode/src/remote-queries/run-remote-query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ interface QueriesResponse {
invalid_repositories?: string[],
repositories_without_database?: string[],
private_repositories?: string[],
cutoff_repositories?: string[],
cutoff_repositories_count?: number,
},
repositories_queried: string[],
}
Expand Down Expand Up @@ -358,18 +360,31 @@ export function parseResponse(owner: string, repo: string, response: QueriesResp
let logMessage = `Successfully scheduled runs on ${numRepositoriesQueried} repositories. See https://github.com/${owner}/${repo}/actions/runs/${response.workflow_run_id}.`;
logMessage += `${eol2}Repositories queried:${eol}${repositoriesQueried.join(', ')}`;
if (response.errors) {
const { invalid_repositories, repositories_without_database, private_repositories, cutoff_repositories, cutoff_repositories_count } = response.errors;
logMessage += `${eol2}Some repositories could not be scheduled.`;
if (response.errors.invalid_repositories?.length) {
logMessage += `${eol2}Invalid repositories:${eol}${response.errors.invalid_repositories.join(', ')}`;
if (invalid_repositories?.length) {
logMessage += `${eol2}${invalid_repositories.length} repositories were invalid and could not be found:${eol}${invalid_repositories.join(', ')}`;
}
if (response.errors.repositories_without_database?.length) {
logMessage += `${eol2}Repositories without databases:${eol}${response.errors.repositories_without_database.join(', ')}`;
if (repositories_without_database?.length) {
logMessage += `${eol2}${repositories_without_database.length} repositories did not have a CodeQL database available:${eol}${repositories_without_database.join(', ')}`;
logMessage += `${eol}For each public repository that has not yet been added to the database service, we will try to create a database next time the store is updated.`;
}
if (response.errors.private_repositories?.length) {
logMessage += `${eol2}Non-public repositories:${eol}${response.errors.private_repositories.join(', ')}`;
if (private_repositories?.length) {
logMessage += `${eol2}${private_repositories.length} repositories are not public:${eol}${private_repositories.join(', ')}`;
Copy link
Copy Markdown
Contributor

@elenatanasoiu elenatanasoiu Jun 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're building a log message from:

  • two end of lines
  • a number
  • a message
  • another eol
  • a list

in three separate situations, extracting those lines into a method might improve readability.

e.g.

logMessage += buildCutOffMessage(private_repositories, "repositories are not public");

let buildCutOffMessage = (list, message) => `${eol2}${list.length} ${message}:${eol}${list.join(', ')}`;

Copy link
Copy Markdown
Contributor Author

@robertbrignull robertbrignull Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this suggestion but I think I'll just get the PR merged first so I can get this and the API change done today. Then potentially come back to this and see how it works out. Definitely the potential to make the area more understandable, but also could spend a lot of time messing with it.

logMessage += `${eol}When using a public controller repository, only public repositories can be queried.`;
}
if (cutoff_repositories_count) {
logMessage += `${eol2}${cutoff_repositories_count} repositories over the limit for a single request`;
if (cutoff_repositories) {
logMessage += `:${eol}${cutoff_repositories.join(', ')}`;
if (cutoff_repositories_count !== cutoff_repositories.length) {
logMessage += `${eol}...${eol}And ${cutoff_repositories_count - cutoff_repositories.length} more repositrories.`;
}
} else {
logMessage += '.';
}
logMessage += `${eol}Repositories were selected based on how recently they had been updated.`;
}
}

return {
Expand Down
66 changes: 61 additions & 5 deletions extensions/ql-vscode/src/vscode-tests/no-workspace/remote-queries/run-remote-query.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('run-remote-query', () => {
'',
'Some repositories could not be scheduled.',
'',
'Invalid repositories:',
'2 repositories were invalid and could not be found:',
'e/f, g/h'].join(os.EOL)
);
});
Expand All @@ -68,7 +68,7 @@ describe('run-remote-query', () => {
'',
'Some repositories could not be scheduled.',
'',
'Repositories without databases:',
'2 repositories did not have a CodeQL database available:',
'e/f, g/h',
'For each public repository that has not yet been added to the database service, we will try to create a database next time the store is updated.'].join(os.EOL)
);
Expand Down Expand Up @@ -96,12 +96,68 @@ describe('run-remote-query', () => {
'',
'Some repositories could not be scheduled.',
'',
'Non-public repositories:',
'2 repositories are not public:',
'e/f, g/h',
'When using a public controller repository, only public repositories can be queried.'].join(os.EOL)
);
});

it('should parse a response with cutoff repos and cutoff repos count', () => {
const result = parseResponse('org', 'name', {
workflow_run_id: 123,
repositories_queried: ['a/b', 'c/d'],
errors: {
cutoff_repositories: ['e/f', 'g/h'],
cutoff_repositories_count: 2,
}
});

expect(result.popupMessage).to.equal(
['Successfully scheduled runs on 2 repositories. [Click here to see the progress](https://github.com/org/name/actions/runs/123).',
'',
'Some repositories could not be scheduled. See extension log for details.'].join(os.EOL)
);
expect(result.logMessage).to.equal(
['Successfully scheduled runs on 2 repositories. See https://github.com/org/name/actions/runs/123.',
'',
'Repositories queried:',
'a/b, c/d',
'',
'Some repositories could not be scheduled.',
'',
'2 repositories over the limit for a single request:',
'e/f, g/h',
'Repositories were selected based on how recently they had been updated.'].join(os.EOL)
);
});

it('should parse a response with cutoff repos count but not cutoff repos', () => {
const result = parseResponse('org', 'name', {
workflow_run_id: 123,
repositories_queried: ['a/b', 'c/d'],
errors: {
cutoff_repositories_count: 2,
}
});

expect(result.popupMessage).to.equal(
['Successfully scheduled runs on 2 repositories. [Click here to see the progress](https://github.com/org/name/actions/runs/123).',
'',
'Some repositories could not be scheduled. See extension log for details.'].join(os.EOL)
);
expect(result.logMessage).to.equal(
['Successfully scheduled runs on 2 repositories. See https://github.com/org/name/actions/runs/123.',
'',
'Repositories queried:',
'a/b, c/d',
'',
'Some repositories could not be scheduled.',
'',
'2 repositories over the limit for a single request.',
'Repositories were selected based on how recently they had been updated.'].join(os.EOL)
);
});

it('should parse a response with invalid repos and repos w/o databases', () => {
const result = parseResponse('org', 'name', {
workflow_run_id: 123,
Expand All @@ -125,10 +181,10 @@ describe('run-remote-query', () => {
'',
'Some repositories could not be scheduled.',
'',
'Invalid repositories:',
'2 repositories were invalid and could not be found:',
'e/f, g/h',
'',
'Repositories without databases:',
'2 repositories did not have a CodeQL database available:',
'i/j, k/l',
'For each public repository that has not yet been added to the database service, we will try to create a database next time the store is updated.'].join(os.EOL)
);
Expand Down