Fixed should be stale condition#304
Merged
hross merged 2 commits intoactions:mainfrom Feb 5, 2021
Merged
Conversation
When different stale period is used for issues and pull requests, this code uses wrong one. This was probably missed in actions#224 Fixes actions#299
nijel
added a commit
to WeblateOrg/meta
that referenced
this pull request
Feb 4, 2021
There seems to be a bug in current actions/stale and it uses the default value for checking actual staleness of an issue. So use the lower value there. This will make marking PRs as stale earlier, but we don't have any stale right now. Hopefully it will get properly fixed in the action. See actions/stale#304 See actions/stale#299
Contributor
|
@nijel thanks for the fix. |
Contributor
Author
|
Yes, it would be good to have this covered by the test, but I can't write them sorry (my typescript knowledge is exactly zero). |
Contributor
|
@nijel ok then I will add some coverage tonight (CET) |
Contributor
|
@nijel test('processing an issue opened since 2 days and with the option "daysBeforeIssueStale" at 3 will not make it stale', async () => {
expect.assertions(2);
const opts: IssueProcessorOptions = {
...DefaultProcessorOptions,
daysBeforeStale: 10,
daysBeforeIssueStale: 3
};
let issueDate = new Date();
issueDate.setDate(issueDate.getDate() - 2);
const TestIssueList: Issue[] = [
generateIssue(
opts,
1,
'An issue with no label',
issueDate.toDateString(),
)
];
const processor = new IssueProcessor(
opts,
async () => 'abot',
async p => (p == 1 ? TestIssueList : []),
async (num, dt) => [],
async (issue, label) => new Date().toDateString()
);
// process our fake issue list
await processor.processIssues(1);
expect(processor.staleIssues.length).toEqual(0);
expect(processor.closedIssues.length).toEqual(0);
});
test('processing an issue opened since 2 days and with the option "daysBeforeIssueStale" at 2 will make it stale', async () => {
expect.assertions(2);
const opts: IssueProcessorOptions = {
...DefaultProcessorOptions,
daysBeforeStale: 10,
daysBeforeIssueStale: 2
};
let issueDate = new Date();
issueDate.setDate(issueDate.getDate() - 2);
const TestIssueList: Issue[] = [
generateIssue(
opts,
1,
'An issue with no label',
issueDate.toDateString(),
)
];
const processor = new IssueProcessor(
opts,
async () => 'abot',
async p => (p == 1 ? TestIssueList : []),
async (num, dt) => [],
async (issue, label) => new Date().toDateString()
);
// process our fake issue list
await processor.processIssues(1);
expect(processor.staleIssues.length).toEqual(1);
expect(processor.closedIssues.length).toEqual(0);
});
test('processing an issue opened since 2 days and with the option "daysBeforeIssueStale" at 1 will make it stale', async () => {
expect.assertions(2);
const opts: IssueProcessorOptions = {
...DefaultProcessorOptions,
daysBeforeStale: 10,
daysBeforeIssueStale: 1
};
let issueDate = new Date();
issueDate.setDate(issueDate.getDate() - 2);
const TestIssueList: Issue[] = [
generateIssue(
opts,
1,
'An issue with no label',
issueDate.toDateString(),
)
];
const processor = new IssueProcessor(
opts,
async () => 'abot',
async p => (p == 1 ? TestIssueList : []),
async (num, dt) => [],
async (issue, label) => new Date().toDateString()
);
// process our fake issue list
await processor.processIssues(1);
expect(processor.staleIssues.length).toEqual(1);
expect(processor.closedIssues.length).toEqual(0);
});
test('processing a pull request opened since 2 days and with the option "daysBeforePrStale" at 3 will not make it stale', async () => {
expect.assertions(2);
const opts: IssueProcessorOptions = {
...DefaultProcessorOptions,
daysBeforeStale: 10,
daysBeforePrStale: 3
};
let issueDate = new Date();
issueDate.setDate(issueDate.getDate() - 2);
const TestIssueList: Issue[] = [
generateIssue(
opts,
1,
'A pull request with no label',
issueDate.toDateString(),
issueDate.toDateString(),
true
)
];
const processor = new IssueProcessor(
opts,
async () => 'abot',
async p => (p == 1 ? TestIssueList : []),
async (num, dt) => [],
async (issue, label) => new Date().toDateString()
);
// process our fake issue list
await processor.processIssues(1);
expect(processor.staleIssues.length).toEqual(0);
expect(processor.closedIssues.length).toEqual(0);
});
test('processing a pull request opened since 2 days and with the option "daysBeforePrStale" at 2 will make it stale', async () => {
expect.assertions(2);
const opts: IssueProcessorOptions = {
...DefaultProcessorOptions,
daysBeforeStale: 10,
daysBeforePrStale: 2
};
let issueDate = new Date();
issueDate.setDate(issueDate.getDate() - 2);
const TestIssueList: Issue[] = [
generateIssue(
opts,
1,
'A pull request with no label',
issueDate.toDateString(),
issueDate.toDateString(),
true
)
];
const processor = new IssueProcessor(
opts,
async () => 'abot',
async p => (p == 1 ? TestIssueList : []),
async (num, dt) => [],
async (issue, label) => new Date().toDateString()
);
// process our fake issue list
await processor.processIssues(1);
expect(processor.staleIssues.length).toEqual(1);
expect(processor.closedIssues.length).toEqual(0);
});
test('processing a pull request opened since 2 days and with the option "daysBeforePrStale" at 1 will make it stale', async () => {
expect.assertions(2);
const opts: IssueProcessorOptions = {
...DefaultProcessorOptions,
daysBeforeStale: 10,
daysBeforePrStale: 1
};
let issueDate = new Date();
issueDate.setDate(issueDate.getDate() - 2);
const TestIssueList: Issue[] = [
generateIssue(
opts,
1,
'A pull request with no label',
issueDate.toDateString(),
issueDate.toDateString(),
true
)
];
const processor = new IssueProcessor(
opts,
async () => 'abot',
async p => (p == 1 ? TestIssueList : []),
async (num, dt) => [],
async (issue, label) => new Date().toDateString()
);
// process our fake issue list
await processor.processIssues(1);
expect(processor.staleIssues.length).toEqual(1);
expect(processor.closedIssues.length).toEqual(0);
});You can add a commit in your PR with this or eventually you could give me the right to push on your fork. |
Contributor
Author
|
Cool, I've added them. |
Contributor
|
@hross hello. I made a mistake with the new options regarding the stale days for an issue or a pull request. The tests are confirming that this PR will make these options useful. If you can take the time to merge and release it it would be very nice. Thanks! |
hross
approved these changes
Feb 5, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When different stale period is used for issues and pull requests, this code uses wrong one.
This was probably missed in #224
Fixes #299
This is not tested at all, but I hope the fix is correct.