mirror of
https://github.com/actions/labeler
synced 2026-05-11 07:11:15 +02:00
Add 'changed-files-labels-limit' and 'max-files-changed' configs to allow capping number of labels added (#923)
* README.md: drop trailing whitespace * Add 'changed-files-labels-limit' config to allow capping number of labels added When a repository has many components, each with a changed-files label, a large refactor ends up with the labeler spamming the pull request with label changes. The end result is not very useful as it's not very readable, and due to how github automatically hides comments when label changes overflow the discussion tab, it means useful information is hidden and one has to manually click "Load more..." dozens of time every time the page is loaded. Add a changed-files-labels-limit top level config knob. If more than the configured limit of labels is set to be added, none are added. This only affects changed-files labels. * Add 'max-files-changed' config to allow capping number of files for labelling When a PR modifies a very large number of files (e.g., tree-wide refactors, automated code formatting), this new options allows skipping file-based labeling entirely when the number of files that are changed hits the configured limit. Fixes https://github.com/actions/labeler/issues/486
This commit is contained in:
Luca Boccassi
GitHub
@@ -9,7 +9,9 @@ import {
|
||||
MatchConfig,
|
||||
toMatchConfig,
|
||||
getLabelConfigMapFromObject,
|
||||
BaseMatchConfig
|
||||
getLabelConfigResultFromObject,
|
||||
BaseMatchConfig,
|
||||
configUsesChangedFiles
|
||||
} from '../src/api/get-label-configs';
|
||||
|
||||
jest.mock('@actions/core');
|
||||
@@ -60,6 +62,204 @@ describe('getLabelConfigMapFromObject', () => {
|
||||
const result = getLabelConfigMapFromObject(yamlObject);
|
||||
expect(result).toEqual(expected);
|
||||
});
|
||||
|
||||
it('ignores top-level options like changed-files-labels-limit and max-files-changed', () => {
|
||||
const configWithLimit = {
|
||||
'changed-files-labels-limit': 5,
|
||||
'max-files-changed': 100,
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigMapFromObject(configWithLimit);
|
||||
expect(result.has('changed-files-labels-limit')).toBe(false);
|
||||
expect(result.has('max-files-changed')).toBe(false);
|
||||
expect(result.has('label1')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getLabelConfigResultFromObject', () => {
|
||||
it('extracts changed-files-labels-limit as a number', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': 5,
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.changedFilesLimit).toBe(5);
|
||||
expect(result.labelConfigs.has('label1')).toBe(true);
|
||||
});
|
||||
|
||||
it('parses changed-files-labels-limit from string', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': '10',
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.changedFilesLimit).toBe(10);
|
||||
});
|
||||
|
||||
it('trims whitespace when parsing string values', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': ' 5 ',
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.changedFilesLimit).toBe(5);
|
||||
});
|
||||
|
||||
it('returns undefined changedFilesLimit when not set', () => {
|
||||
const config = {
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.changedFilesLimit).toBeUndefined();
|
||||
});
|
||||
|
||||
it('throws error for invalid changed-files-labels-limit value', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': 'invalid',
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
expect(() => getLabelConfigResultFromObject(config)).toThrow(
|
||||
/Invalid value for 'changed-files-labels-limit'/
|
||||
);
|
||||
});
|
||||
|
||||
it('throws error for negative changed-files-labels-limit value', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': -1,
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
expect(() => getLabelConfigResultFromObject(config)).toThrow(
|
||||
/must be a non-negative integer/
|
||||
);
|
||||
});
|
||||
|
||||
it('throws error for string with trailing characters', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': '10abc',
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
expect(() => getLabelConfigResultFromObject(config)).toThrow(
|
||||
/must be a non-negative integer/
|
||||
);
|
||||
});
|
||||
|
||||
it('throws error for decimal string', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': '3.2',
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
expect(() => getLabelConfigResultFromObject(config)).toThrow(
|
||||
/must be a non-negative integer/
|
||||
);
|
||||
});
|
||||
|
||||
it('throws error for float number', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': 3.2,
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
expect(() => getLabelConfigResultFromObject(config)).toThrow(
|
||||
/must be a non-negative integer/
|
||||
);
|
||||
});
|
||||
|
||||
it('accepts zero as a valid changed-files-labels-limit', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': 0,
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.changedFilesLimit).toBe(0);
|
||||
});
|
||||
|
||||
it('extracts max-files-changed as a number', () => {
|
||||
const config = {
|
||||
'max-files-changed': 100,
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.maxFilesChanged).toBe(100);
|
||||
expect(result.labelConfigs.has('label1')).toBe(true);
|
||||
});
|
||||
|
||||
it('parses max-files-changed from string', () => {
|
||||
const config = {
|
||||
'max-files-changed': '50',
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.maxFilesChanged).toBe(50);
|
||||
});
|
||||
|
||||
it('returns undefined maxFilesChanged when not set', () => {
|
||||
const config = {
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.maxFilesChanged).toBeUndefined();
|
||||
});
|
||||
|
||||
it('throws error for invalid max-files-changed value', () => {
|
||||
const config = {
|
||||
'max-files-changed': 'invalid',
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
expect(() => getLabelConfigResultFromObject(config)).toThrow(
|
||||
/Invalid value for 'max-files-changed'/
|
||||
);
|
||||
});
|
||||
|
||||
it('throws error for negative max-files-changed value', () => {
|
||||
const config = {
|
||||
'max-files-changed': -1,
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
expect(() => getLabelConfigResultFromObject(config)).toThrow(
|
||||
/must be a non-negative integer/
|
||||
);
|
||||
});
|
||||
|
||||
it('accepts zero as a valid max-files-changed', () => {
|
||||
const config = {
|
||||
'max-files-changed': 0,
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.maxFilesChanged).toBe(0);
|
||||
});
|
||||
|
||||
it('supports both options together', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': 5,
|
||||
'max-files-changed': 100,
|
||||
label1: [{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}]
|
||||
};
|
||||
const result = getLabelConfigResultFromObject(config);
|
||||
expect(result.changedFilesLimit).toBe(5);
|
||||
expect(result.maxFilesChanged).toBe(100);
|
||||
});
|
||||
|
||||
it('throws a clear error when max-files-changed is used as a label', () => {
|
||||
const config = {
|
||||
'max-files-changed': [
|
||||
{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}
|
||||
]
|
||||
};
|
||||
expect(() => getLabelConfigResultFromObject(config)).toThrow(
|
||||
/reserved top-level option and cannot be used as a label name/
|
||||
);
|
||||
});
|
||||
|
||||
it('throws a clear error when changed-files-labels-limit is used as a label', () => {
|
||||
const config = {
|
||||
'changed-files-labels-limit': [
|
||||
{'changed-files': [{'any-glob-to-any-file': ['*.txt']}]}
|
||||
]
|
||||
};
|
||||
expect(() => getLabelConfigResultFromObject(config)).toThrow(
|
||||
/reserved top-level option and cannot be used as a label name/
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('toMatchConfig', () => {
|
||||
@@ -164,6 +364,52 @@ describe('checkMatchConfigs', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('configUsesChangedFiles', () => {
|
||||
it('returns true when config has changed-files in any block', () => {
|
||||
const matchConfig: MatchConfig[] = [
|
||||
{any: [{changedFiles: [{anyGlobToAnyFile: ['*.txt']}]}]}
|
||||
];
|
||||
expect(configUsesChangedFiles(matchConfig)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true when config has changed-files in all block', () => {
|
||||
const matchConfig: MatchConfig[] = [
|
||||
{all: [{changedFiles: [{allGlobsToAllFiles: ['*.txt']}]}]}
|
||||
];
|
||||
expect(configUsesChangedFiles(matchConfig)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false when config only has branch patterns', () => {
|
||||
const matchConfig: MatchConfig[] = [
|
||||
{any: [{headBranch: ['^test/']}]},
|
||||
{any: [{baseBranch: ['main']}]}
|
||||
];
|
||||
expect(configUsesChangedFiles(matchConfig)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when config has empty changed-files array', () => {
|
||||
const matchConfig: MatchConfig[] = [{any: [{changedFiles: []}]}];
|
||||
expect(configUsesChangedFiles(matchConfig)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when config has changed-files with empty objects', () => {
|
||||
const matchConfig: MatchConfig[] = [{any: [{changedFiles: [{}]}]}];
|
||||
expect(configUsesChangedFiles(matchConfig)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true when config has mixed branch and changed-files patterns', () => {
|
||||
const matchConfig: MatchConfig[] = [
|
||||
{
|
||||
any: [
|
||||
{changedFiles: [{anyGlobToAnyFile: ['*.txt']}]},
|
||||
{headBranch: ['^feature/']}
|
||||
]
|
||||
}
|
||||
];
|
||||
expect(configUsesChangedFiles(matchConfig)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('labeler error handling', () => {
|
||||
const mockClient = {} as any;
|
||||
const mockPullRequest = {
|
||||
@@ -183,9 +429,10 @@ describe('labeler error handling', () => {
|
||||
}
|
||||
]);
|
||||
|
||||
(api.getLabelConfigs as jest.Mock).mockResolvedValue(
|
||||
new Map([['new-label', ['dummy-config']]])
|
||||
);
|
||||
(api.getLabelConfigs as jest.Mock).mockResolvedValue({
|
||||
labelConfigs: new Map([['new-label', ['dummy-config']]]),
|
||||
changedFilesLimit: undefined
|
||||
});
|
||||
|
||||
// Force match so "new-label" is always added
|
||||
jest.spyOn({checkMatchConfigs}, 'checkMatchConfigs').mockReturnValue(true);
|
||||
|
||||
Reference in New Issue
Block a user