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.

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

feat: [auto-approve] add python sample dependency process #5002

Merged
merged 7 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions packages/auto-approve/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ processes:
- "RegenerateReadme"
- "DiscoveryDocUpdate"
- "PythonDependency"
- "PythonSampleDependency"
- "NodeDependency"
- "NodeRelease"
- "JavaApiaryCodegen"
Expand Down Expand Up @@ -61,6 +62,17 @@ Below is what each process checks for:
- Increase the non-major package version of a dependency
- Only change one dependency
- Change the dependency that was there previously, and that is on the title of the PR
* PythonSampleDependency:
- Checks that the author is 'renovate-bot'
- Checks that the title of the PR matches the regexp: /^(fix|chore)\(deps\): update dependency (@?\S*) to v(\S*)$/
- Each file path must match one of these regexps:
- /requirements.txt$/
- All files must:
- Match this regexp: /requirements.txt$/
- Increase the non-major package version of a dependency
- Only change one dependency, that must be a google dependency
- Change the dependency that was there previously, and that is on the title of the PR
- Not match any regexes in the 'excluded' list
* NodeDependency:
- Checks that the author is 'renovate-bot'
- Checks that the title of the PR matches the regexp: /^(fix|chore)\(deps\): update dependency (@?\S*) to v(\S*)$/
Expand Down
2 changes: 2 additions & 0 deletions packages/auto-approve/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ export interface FileSpecificRule {
* regex for the versions for those particular formats.
*/
export interface FileRule {
targetFileToExclude?: RegExp[];
oldVersion?: RegExp;
newVersion?: RegExp;
dependencyTitle?: RegExp;
targetFileToCheck: RegExp;
regexForDepToInclude?: RegExp[];
}

/**
Expand Down
198 changes: 198 additions & 0 deletions packages/auto-approve/src/process-checks/python/sample-dependency.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
// Copyright 2023 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import {LanguageRule, File, FileRule, Process} from '../../interfaces';
import {
checkAuthor,
checkTitleOrBody,
checkFilePathsMatch,
doesDependencyChangeMatchPRTitleV2,
getVersionsV2,
runVersioningValidation,
isOneDependencyChanged,
reportIndividualChecks,
doesDependencyMatchAgainstRegexes,
} from '../../utils-for-pr-checking';
import {Octokit} from '@octokit/rest';

export class PythonSampleDependency extends Process implements LanguageRule {
classRule: {
author: string;
titleRegex?: RegExp;
fileNameRegex?: RegExp[];
fileRules?: {
oldVersion?: RegExp;
newVersion?: RegExp;
dependencyTitle?: RegExp;
targetFileToCheck: RegExp;
targetFileToExclude: RegExp[];
regexForDepToInclude: RegExp[];
}[];
};

constructor(
incomingPrAuthor: string,
incomingTitle: string,
incomingFileCount: number,
incomingChangedFiles: File[],
incomingRepoName: string,
incomingRepoOwner: string,
incomingPrNumber: number,
incomingOctokit: Octokit,
incomingBody?: string
) {
super(
incomingPrAuthor,
incomingTitle,
incomingFileCount,
incomingChangedFiles,
incomingRepoName,
incomingRepoOwner,
incomingPrNumber,
incomingOctokit,
incomingBody
),
(this.classRule = {
author: 'renovate-bot',
titleRegex:
/^(fix|chore)\(deps\): update dependency (@?\S*) to v(\S*)$/,
fileNameRegex: [/requirements.txt$/],
fileRules: [
{
targetFileToCheck: /requirements.txt$/,
// @Python team: please add API paths here to exclude from auto-approving
targetFileToExclude: [/airflow/, /composer/],
// This would match: fix(deps): update dependency @octokit to v1
dependencyTitle: new RegExp(
/^(fix|chore)\(deps\): update dependency (@?\S*) to v(\S*)$/
),
// This would match: '-google-cloud-storage==1.39.0
oldVersion: new RegExp(
/[\s]-(@?[^=0-9]*)==([0-9])*\.([0-9]*\.[0-9]*)/
),
// This would match: '+google-cloud-storage==1.40.0
newVersion: new RegExp(
/[\s]\+(@?[^=0-9]*)==([0-9])*\.([0-9]*\.[0-9]*)/
),
regexForDepToInclude: [/google/],
},
],
});
}

public async checkPR(): Promise<boolean> {
const authorshipMatches = checkAuthor(
this.classRule.author,
this.incomingPR.author
);

const titleMatches = checkTitleOrBody(
this.incomingPR.title,
this.classRule.titleRegex
);

const filePatternsMatch = checkFilePathsMatch(
this.incomingPR.changedFiles.map(x => x.filename),
this.classRule.fileNameRegex
);

for (const file of this.incomingPR.changedFiles) {
// Each file must conform to at least one file rule, or else we could
// be allowing a random file to be approved
const fileMatch = this.classRule.fileRules?.find((x: FileRule) =>
x.targetFileToCheck.test(file.filename)
);

if (fileMatch?.targetFileToExclude) {
// Can disable the error message below because we are checking to see if
// fileMatch.targetFileToExclude exists first
// eslint-disable-next-line no-unsafe-optional-chaining
for (const targetFilesToExclude of fileMatch?.targetFileToExclude) {
// If any file contains an excluded name, exit out immediately
if (targetFilesToExclude.test(file.filename)) {
return false;
}
}
}

if (!fileMatch) {
return false;
}

const versions = getVersionsV2(
file,
fileMatch.oldVersion,
fileMatch.newVersion
);

if (!versions) {
return false;
}

const doesDependencyMatch = doesDependencyChangeMatchPRTitleV2(
versions,
// We can assert this exists since we're in the class rule that contains it
fileMatch.dependencyTitle!,
this.incomingPR.title
);

const doesDependencyConformToRegexes = doesDependencyMatchAgainstRegexes(
versions,
fileMatch.regexForDepToInclude
);

const isVersionValid = runVersioningValidation(versions);

const oneDependencyChanged = isOneDependencyChanged(file);

if (
!(
doesDependencyMatch &&
isVersionValid &&
oneDependencyChanged &&
doesDependencyConformToRegexes
)
) {
reportIndividualChecks(
[
'doesDependencyMatch',
'isVersionValid',
'oneDependencyChanged',
'doesDependencyConformToRegexes',
],
[
doesDependencyMatch,
isVersionValid,
oneDependencyChanged,
doesDependencyConformToRegexes,
],
this.incomingPR.repoOwner,
this.incomingPR.repoName,
this.incomingPR.prNumber,
file.filename
);
return false;
}
}

reportIndividualChecks(
['authorshipMatches', 'titleMatches', 'filePatternsMatch'],
[authorshipMatches, titleMatches, filePatternsMatch],
this.incomingPR.repoOwner,
this.incomingPR.repoName,
this.incomingPR.prNumber
);
return authorshipMatches && titleMatches && filePatternsMatch;
}
}
35 changes: 26 additions & 9 deletions packages/auto-approve/src/utils-for-pr-checking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,28 @@ export function doesDependencyChangeMatchPRTitle(
}

/**
* This function checks whether the dependency stated in a given title was the one that was changed (non Java, see doesDependencyChangeMatchPRTitleJava)
* This function checks whether the dependency changed matches a regex to include
*
* @param versions the Versions object that contains the old dependency name and new dependency name and versions
* @param regexToInclude an array of regexes to search for
* @returns whether the dependencies match to any of the given regexesß
*/
export function doesDependencyMatchAgainstRegexes(
versions: Versions,
regexToInclude: RegExp[]
): boolean {
let doesDepIncludeRegexToInclude = false;
regexToInclude?.forEach(regex => {
if (versions.newDependencyName.match(regex)) {
doesDepIncludeRegexToInclude = true;
}
});

return doesDepIncludeRegexToInclude;
}

/**
* This function checks whether the dependency stated in a given title was the one that was changed (not Java, see doesDependencyChangeMatchPRTitleJava)
*
* @param versions the Versions object that contains the old dependency name and new dependency name and versions
* @param dependencyRegex the regular exp to find the dependency within the title of the PR
Expand All @@ -262,16 +283,12 @@ export function doesDependencyChangeMatchPRTitleV2(

if (titleRegex) {
dependencyName = titleRegex[2];

return (
versions.newDependencyName === versions.oldDependencyName &&
dependencyName === versions.newDependencyName
);
}

return false;
return (
versions.newDependencyName === versions.oldDependencyName &&
dependencyName === versions.newDependencyName
);
}

/**
* This function determines whether the major version of a package was changed.
*
Expand Down
Loading