fix(auto-approve): update PythonSampleDependency for python team (goo…

…gleapis#5266)

See "[Implementation] Auto-approve Python docs samples"

After discussion, will fix googleapis#5241 per conversation

cc @msampathkumar

packages/auto-approve/README.md

@@ -69,14 +69,15 @@ Below is what each process checks for:
     - 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 author is 'renovate-bot' or 'dependabot'
   - Checks that the title of the PR matches the regexp: /^(fix|chore)\(deps\): update dependency (@?\S*) to v(\S*)$/
+  or /^(chore)\(deps\): bump (@?\S*) from \S* to (\S*) in \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
+    - Only change one 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:

packages/auto-approve/src/auto-approve.ts

@@ -379,7 +379,18 @@ export function handler(app: Probot) {
               );
             }
 
-            await retryAddLabel(3, owner, repo, prNumber, octokit);
+            // Currently, the python-docs-samples team do not want to automerge
+            // the PRs. I'm hardcoding their exception since this hasn't otherwise
+            // been a feature request, and they are expecting to want to merge in the
+            // future.
+            if (
+              !(
+                owner === 'GoogleCloudPlatform' &&
+                repo === 'python-docs-samples'
+              )
+            ) {
+              await retryAddLabel(3, owner, repo, prNumber, octokit);
+            }
           } else if (isConfigValid && !isPRValid) {
             // If config is valid but PR isn't, log that it is not valid, but don't comment on PR since that would be noisy
             logger.info(

packages/auto-approve/src/process-checks/python/sample-dependency.ts

@@ -30,21 +30,25 @@ import {BaseLanguageRule} from '../base';
 /**
  * The PythonSampleDependency class's checkPR function returns
  * true if the PR:
-  - has an author that is 'renovate-bot'
+  - has an author that is 'renovate-bot' or 'dependabot'
   - has a title that matches the regexp: /^(fix|chore)\(deps\): update dependency (@?\S*) to v(\S*)$/
+    or /^(chore)\(deps\): bump (@?\S*) from \S* to (\S*) in \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
+    - Only change one 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
  */
 export class PythonSampleDependency extends BaseLanguageRule {
   classRule = {
-    author: 'renovate-bot',
-    titleRegex: /^(fix|chore)\(deps\): update dependency (@?\S*) to v(\S*)$/,
+    authors: ['renovate-bot', 'dependabot'],
+    titleRegex: [
+      /^(fix|chore)\(deps\): update dependency (@?\S*) to v(\S*)$/,
+      /^(chore)\(deps\): bump (@?\S*) from \S* to (\S*) in \S*/,
+    ],
     fileNameRegex: [/requirements.txt$/],
   };
   fileRules = [
@@ -53,14 +57,15 @@ export class PythonSampleDependency extends BaseLanguageRule {
       // @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*)$/
-      ),
+      dependencyTitles: [
+        /^(fix|chore)\(deps\): update dependency (@?\S*) to v(\S*)$/,
+        /^(chore)\(deps\): bump (@?\S*) from \S* to v?([0-9]*)\.([0-9]*\.?[0-9]*) in \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/],
+      regexForDepToNotInclude: [/airflow/, /composer/, /secretmanager/],
     },
   ];
 
@@ -69,15 +74,19 @@ export class PythonSampleDependency extends BaseLanguageRule {
   }
 
   public async checkPR(incomingPR: PullRequest): Promise<boolean> {
-    const authorshipMatches = checkAuthor(
-      this.classRule.author,
-      incomingPR.author
-    );
+    let authorshipMatches = false;
+    for (const author of this.classRule.authors) {
+      if (checkAuthor(author, incomingPR.author)) {
+        authorshipMatches = true;
+      }
+    }
 
-    const titleMatches = checkTitleOrBody(
-      incomingPR.title,
-      this.classRule.titleRegex
-    );
+    let titleMatches = false;
+    for (const titleRegex of this.classRule.titleRegex) {
+      if (checkTitleOrBody(incomingPR.title, titleRegex)) {
+        titleMatches = true;
+      }
+    }
 
     const filePatternsMatch = checkFilePathsMatch(
       incomingPR.changedFiles.map(x => x.filename),
@@ -117,16 +126,23 @@ export class PythonSampleDependency extends BaseLanguageRule {
         return false;
       }
 
-      const doesDependencyMatch = doesDependencyChangeMatchPRTitleV2(
-        versions,
-        // We can assert this exists since we're in the class rule that contains it
-        fileMatch.dependencyTitle!,
-        incomingPR.title
-      );
+      let doesDependencyMatch = false;
+      for (const dependencyTitle of fileMatch.dependencyTitles) {
+        if (
+          doesDependencyChangeMatchPRTitleV2(
+            versions,
+            // We can assert this exists since we're in the class rule that contains it
+            dependencyTitle,
+            incomingPR.title
+          )
+        ) {
+          doesDependencyMatch = true;
+        }
+      }
 
-      const doesDependencyConformToRegexes = doesDependencyMatchAgainstRegexes(
+      const doesDependencyConformToRegexes = !doesDependencyMatchAgainstRegexes(
         versions,
-        fileMatch.regexForDepToInclude
+        fileMatch.regexForDepToNotInclude
       );
 
       const isVersionValid = runVersioningValidation(versions);

packages/auto-approve/src/utils-for-pr-checking.ts

@@ -298,9 +298,12 @@ export function doesDependencyChangeMatchPRTitleV2(
   if (titleRegex) {
     dependencyName = titleRegex[2];
   }
+
   return (
-    versions.newDependencyName === versions.oldDependencyName &&
-    dependencyName === versions.newDependencyName
+    versions.newDependencyName.toLocaleLowerCase() ===
+      versions.oldDependencyName.toLocaleLowerCase() &&
+    dependencyName.toLocaleLowerCase() ===
+      versions.newDependencyName.toLocaleLowerCase()
   );
 }
 
@@ -321,7 +324,13 @@ export function isMajorVersionChanging(versions: Versions): boolean {
  * @returns whether the minor version was upgraded.
  */
 export function isMinorVersionUpgraded(versions: Versions): boolean {
-  return Number(versions.newMinorVersion) > Number(versions.oldMinorVersion);
+  const [oldMinor, oldPatch] = versions.oldMinorVersion.split('.');
+  const [newMinor, newPatch] = versions.newMinorVersion.split('.');
+  return (
+    (Number(newMinor) > Number(oldMinor) ||
+      Number(newPatch) > Number(oldPatch)) &&
+    Number(versions.oldMinorVersion) !== Number(versions.newMinorVersion)
+  );
 }
 
 /**

packages/auto-approve/test/auto-approve.test.ts

@@ -267,15 +267,19 @@ describe('auto-approve', () => {
         const scopes = [
           getConfigFile(
             'GoogleCloudPlatform',
-            'python-docs-samples',
+            'python-docs-samples-1',
             {rules: [{author: 'fake-author', title: 'fake-title'}]},
             200
           ),
-          getReviewsCompleted('GoogleCloudPlatform', 'python-docs-samples', []),
-          submitReview('GoogleCloudPlatform', 'python-docs-samples', 200),
-          listLabels('GoogleCloudPlatform', 'python-docs-samples', 200),
-          addLabels('GoogleCloudPlatform', 'python-docs-samples', 200),
-          createCheck('GoogleCloudPlatform', 'python-docs-samples', 200),
+          getReviewsCompleted(
+            'GoogleCloudPlatform',
+            'python-docs-samples-1',
+            []
+          ),
+          submitReview('GoogleCloudPlatform', 'python-docs-samples-1', 200),
+          listLabels('GoogleCloudPlatform', 'python-docs-samples-1', 200),
+          addLabels('GoogleCloudPlatform', 'python-docs-samples-1', 200),
+          createCheck('GoogleCloudPlatform', 'python-docs-samples-1', 200),
         ];
 
         await probot.receive({

packages/auto-approve/test/check-pr-V2.test.ts

@@ -104,16 +104,16 @@ describe('check pr against config', async () => {
       const scopes = [
         getFilesOnAPR(
           'GoogleCloudPlatform',
-          'python-docs-samples',
+          'python-docs-samples-1',
           [{filename: 'requirements.txt', sha: '1234'}],
           200
         ),
-        listCommitsOnAPR('GoogleCloudPlatform', 'python-docs-samples', [
+        listCommitsOnAPR('GoogleCloudPlatform', 'python-docs-samples-1', [
           {author: {login: 'gcf-owl-bot[bot]'}},
         ]),
         getFileOnARepo(
           'GoogleCloudPlatform',
-          'python-docs-samples',
+          'python-docs-samples-1',
           '.repo-metadata.json',
           {
             name: '.repo-metadata.json',
@@ -123,15 +123,15 @@ describe('check pr against config', async () => {
         ),
         getFileOnARepo(
           'GoogleCloudPlatform',
-          'python-docs-samples',
+          'python-docs-samples-1',
           '.repo-metadata.json',
           {
             name: '.repo-metadata.json',
             content:
               'ewogICAgIm5hbWUiOiAiZGxwIiwKICAgICJuYW1lX3ByZXR0eSI6ICJDbG91ZCBEYXRhIExvc3MgUHJldmVudGlvbiIsCiAgICAicHJvZHVjdF9kb2N1bWVudGF0aW9uIjogImh0dHBzOi8vY2xvdWQuZ29vZ2xlLmNvbS9kbHAvZG9jcy8iLAogICAgImNsaWVudF9kb2N1bWVudGF0aW9uIjogImh0dHBzOi8vY2xvdWQuZ29vZ2xlLmNvbS9weXRob24vZG9jcy9yZWZlcmVuY2UvZGxwL2xhdGVzdCIsCiAgICAiaXNzdWVfdHJhY2tlciI6ICIiLAogICAgInJlbGVhc2VfbGV2ZWwiOiAiZ2EiLAogICAgImxhbmd1YWdlIjogInB5dGhvbiIsCiAgICAibGlicmFyeV90eXBlIjogIkdBUElDX0FVVE8iLAogICAgInJlcG8iOiAiZ29vZ2xlYXBpcy9weXRob24tZGxwIiwKICAgICJkaXN0cmlidXRpb25fbmFtZSI6ICJnb29nbGUtY2xvdWQtZGxwIiwKICAgICJhcGlfaWQiOiAiZGxwLmdvb2dsZWFwaXMuY29tIiwKICAgICJyZXF1aXJlc19iaWxsaW5nIjogdHJ1ZSwKICAgICJkZWZhdWx0X3ZlcnNpb24iOiAidjIiLAogICAgImNvZGVvd25lcl90ZWFtIjogIiIKfQ==',
           }
         ),
-        getPRsOnRepo('GoogleCloudPlatform', 'python-docs-samples', [
+        getPRsOnRepo('GoogleCloudPlatform', 'python-docs-samples-1', [
           {id: 2, user: {login: 'gcf-owl-bot[bot]'}},
         ]),
       ];
@@ -156,7 +156,7 @@ describe('check pr against config', async () => {
       const scopes = [
         getFilesOnAPR(
           'GoogleCloudPlatform',
-          'python-docs-samples',
+          'python-docs-samples-1',
           undefined,
           404
         ),

packages/auto-approve/test/check-pr.test.ts

@@ -136,7 +136,7 @@ describe('check pr against config', async () => {
       ) as {rules: ValidPr[]};
 
       const fileRequest = nock('https://api.github.com')
-        .get('/repos/GoogleCloudPlatform/python-docs-samples/pulls/1/files')
+        .get('/repos/GoogleCloudPlatform/python-docs-samples-1/pulls/1/files')
         .reply(200, [{filename: 'requirements.txt', sha: '1234'}]);
 
       const pr = require(resolve(

packages/auto-approve/test/fixtures/events/pull_request_opened_fork.json

@@ -292,8 +292,8 @@
          "repo":{
             "id":35065876,
             "node_id":"MDEwOlJlcG9zaXRvcnkzNTA2NTg3Ng==",
-            "name":"python-docs-samples",
-            "full_name":"GoogleCloudPlatform/python-docs-samples",
+            "name":"python-docs-samples-1",
+            "full_name":"GoogleCloudPlatform/python-docs-samples-1",
             "private":false,
             "owner":{
                "login":"GoogleCloudPlatform",
@@ -441,8 +441,8 @@
    "repository":{
       "id":35065876,
       "node_id":"MDEwOlJlcG9zaXRvcnkzNTA2NTg3Ng==",
-      "name":"python-docs-samples",
-      "full_name":"GoogleCloudPlatform/python-docs-samples",
+      "name":"python-docs-samples-1",
+      "full_name":"GoogleCloudPlatform/python-docs-samples-1",
       "private":false,
       "owner":{
          "login":"GoogleCloudPlatform",

packages/auto-approve/test/python-sample-dependency.test.ts

@@ -113,23 +113,23 @@ describe('behavior of Python Sample Dependency process', () => {
     assert.deepStrictEqual(await pythonDependency.checkPR(incomingPR), false);
   });
 
-  it('should return false if the dependency changed is not a google dependency', async () => {
+  it('should return false if the dependency changed is in the exclude list', async () => {
     const incomingPR = {
       author: 'renovate-bot',
       title: 'chore(deps): update dependency django-environ to v0.9.0',
       fileCount: 3,
       changedFiles: [
         {
           sha: '1349c83bf3c20b102da7ce85ebd384e0822354f3',
-          filename: 'composer/workflows/requirements.txt',
+          filename: 'workflows/requirements.txt',
           additions: 1,
           deletions: 1,
           changes: 2,
           patch:
             '@@ -1,2 +1,2 @@\n' +
             ' google-cloud-videointelligence==2.5.1\n' +
-            '-django-environ==0.7.0\n' +
-            '+django-environ==0.9.0',
+            '-airflow==0.7.0\n' +
+            '+airflow==0.9.0',
         },
       ],
       repoName: 'testRepoName',
@@ -172,4 +172,47 @@ describe('behavior of Python Sample Dependency process', () => {
 
     assert.ok(await pythonDependency.checkPR(incomingPR));
   });
+
+  it('should return true in checkPR if incoming PR matches for dependabot', async () => {
+    const incomingPR = {
+      author: 'dependabot',
+      title:
+        'chore(deps): bump django from 4.1.7 to 4.1.13 in /appengine/standard_python3/bundled-services/blobstore/django',
+      fileCount: 1,
+      changedFiles: [
+        {
+          sha: '7238a9cd2d395d453b1ebb6278440a99574e055e',
+          filename:
+            'appengine/standard_python3/bundled-services/blobstore/django/requirements.txt',
+          status: 'modified',
+          additions: 1,
+          deletions: 1,
+          changes: 2,
+          blob_url:
+            'https://github.com/GoogleCloudPlatform/python-docs-samples/blob/57fe78fbf19010405481f129646fa16904a4b413/appengine%2Fstandard_python3%2Fbundled-services%2Fblobstore%2Fdjango%2Frequirements.txt',
+          raw_url:
+            'https://github.com/GoogleCloudPlatform/python-docs-samples/raw/57fe78fbf19010405481f129646fa16904a4b413/appengine%2Fstandard_python3%2Fbundled-services%2Fblobstore%2Fdjango%2Frequirements.txt',
+          contents_url:
+            'https://api.github.com/repos/GoogleCloudPlatform/python-docs-samples/contents/appengine%2Fstandard_python3%2Fbundled-services%2Fblobstore%2Fdjango%2Frequirements.txt?ref=57fe78fbf19010405481f129646fa16904a4b413',
+          patch:
+            '@@ -1,5 +1,5 @@\n' +
+            ' Django==3.2.18; python_version<"3.8"\n' +
+            '-Django==4.1.7; python_version>"3.7"\n' +
+            '+Django==4.1.13; python_version>"3.7"\n' +
+            ' django-environ==0.10.0\n' +
+            ' google-cloud-logging==3.5.0\n' +
+            ' appengine-python-standard>=0.2.3\n' +
+            '\\ No newline at end of file',
+        },
+      ],
+      repoName: 'testRepoName',
+      repoOwner: 'testRepoOwner',
+      prNumber: 1,
+      body: 'body',
+    };
+
+    const pythonDependency = new PythonSampleDependency(octokit);
+
+    assert.ok(await pythonDependency.checkPR(incomingPR));
+  });
 });