Skip to content
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

Jump to bottom

Updating successful label removal only after the actual Github call is successful #1180

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Updating successful label removal only after the actual Github call is successful #1180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
efe535e
Consider label removed only if the operation was successful
galabra Sep 25, 2024
59ce61f
Added test
galabra Sep 25, 2024
451a0cf
Also moved consumed issues counter
galabra Sep 25, 2024
e6d0d7b
Do consume operation even if query was unsuccessful
galabra Sep 25, 2024
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
39 changes: 39 additions & 0 deletions __tests__/remove-stale-when-updated.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,28 @@ import {DefaultProcessorOptions} from './constants/default-processor-options';
import {generateIssue} from './functions/generate-issue';
import {alwaysFalseStateMock} from './classes/state-mock';

jest.mock('@actions/github', () => {
const actual = jest.requireActual('@actions/github');
return {
...actual,
context: {
...actual.context,
repo: {
owner: 'owner-mock',
repo: 'repo-mock'
}
},
getOctokit: (...args: unknown[]) => {
const originalCall = actual.getOctokit;
const client = originalCall(...args);
client.rest.issues.removeLabel = () => {
throw new Error('Something went wrong');
};
return client;
}
};
});

let issuesProcessorBuilder: IssuesProcessorBuilder;
let issuesProcessor: IssuesProcessorMock;

Expand Down Expand Up @@ -65,6 +87,18 @@ describe('remove-stale-when-updated option', (): void => {

expect(issuesProcessor.removedLabelIssues).toHaveLength(1);
});

test('should not count stale label removal when the operation is unsuccessful', async (): Promise<void> => {
expect.assertions(1);
issuesProcessor = issuesProcessorBuilder
.staleIssues([{}])
.unsetDebugMode()
.build();

await issuesProcessor.processIssues();

expect(issuesProcessor.removedLabelIssues).toHaveLength(0);
});
});
});

Expand Down Expand Up @@ -541,6 +575,11 @@ class IssuesProcessorBuilder {
return this;
}

unsetDebugMode() {
this._options.debugOnly = false;
return this;
}

build(): IssuesProcessorMock {
return new IssuesProcessorMock(
this._options,
Expand Down
5 changes: 3 additions & 2 deletions src/classes/issues-processor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,11 +1016,9 @@ export class IssuesProcessor {
isSubStep ? LoggerService.white('├── ') : ''
}Removing the label "${LoggerService.cyan(label)}" from this $$type...`
);
this.removedLabelIssues.push(issue);

try {
this._consumeIssueOperation(issue);
this.statistics?.incrementDeletedItemsLabelsCount(issue);

if (!this.options.debugOnly) {
await this.client.rest.issues.removeLabel({
Expand All @@ -1036,6 +1034,9 @@ export class IssuesProcessor {
isSubStep ? LoggerService.white('└── ') : ''
}The label "${LoggerService.cyan(label)}" was removed`
);

this.statistics?.incrementDeletedItemsLabelsCount(issue);
this.removedLabelIssues.push(issue);
} catch (error) {
issueLogger.error(
`${
Expand Down