Fix most tests

hub4j · Sep 13, 2024 · 0875baf · 0875baf
1 parent c359119
commit 0875baf
Show file tree

Hide file tree

Showing 9 changed files with 142 additions and 118 deletions.
diff --git a/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java b/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java
@@ -135,15 +135,17 @@ public void onError(GitHubConnectorResponse connectorResponse) throws IOExceptio
         }
     };
 
+    // If "Retry-After" missing, wait for unambiguously over one minute per GitHub guidance
+    static long DEFAULT_WAIT_MILLIS = 61 * 1000;
+
     /*
      * Exposed for testability. Given an http response, find the retry-after header field and parse it as either a
      * number or a date (the spec allows both). If no header is found, wait for a reasonably amount of time.
      */
     static long parseWaitTime(GitHubConnectorResponse connectorResponse) {
         String v = connectorResponse.header("Retry-After");
         if (v == null) {
-            // can't tell, wait for unambiguously over one minute per GitHub guidance
-            return 61 * 1000;
+            return DEFAULT_WAIT_MILLIS;
         }
 
         try {

diff --git a/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java b/src/test/java/org/kohsuke/github/AbuseLimitHandlerTest.java
@@ -340,16 +340,6 @@ public void testHandler_Wait_Secondary_Limits() throws Exception {
         // Customized response that templates the date to keep things working
         snapshotNotAllowed();
 
-        gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl())
-                .withAbuseLimitHandler(GitHubAbuseLimitHandler.WAIT)
-                .build();
-
-        gitHub.getMyself();
-        assertThat(mockGitHub.getRequestCount(), equalTo(1));
-
-        getTempRepository();
-        assertThat(mockGitHub.getRequestCount(), equalTo(3));
-
         gitHub = getGitHubBuilder().withEndpoint(mockGitHub.apiServer().baseUrl())
                 .withAbuseLimitHandler(new GitHubAbuseLimitHandler() {
                     /**
@@ -369,7 +359,7 @@ public void onError(@NotNull GitHubConnectorResponse connectorResponse) throws I
                         assertThat(connectorResponse.request().url().toString(),
                                 endsWith("/repos/hub4j-test-org/temp-testHandler_Wait_Secondary_Limits"));
                         assertThat(connectorResponse.header("X-RateLimit-Limit"), equalTo("5000"));
-                        assertThat(connectorResponse.header("X-RateLimit-Remaining"), equalTo(4000));
+                        assertThat(connectorResponse.header("X-RateLimit-Remaining"), equalTo("4000"));
                         assertThat(connectorResponse.header("X-Foo"), is(nullValue())); // equalTo(20));
                         assertThat(connectorResponse.header("gh-limited-by"),
                                 equalTo("search-elapsed-time-shared-grouped"));
@@ -378,25 +368,27 @@ public void onError(@NotNull GitHubConnectorResponse connectorResponse) throws I
                         // assertThat(uc.getContentLength(), equalTo(-1));
                         assertThat(connectorResponse.allHeaders(), instanceOf(Map.class));
                         assertThat(connectorResponse.allHeaders().size(), greaterThan(25));
+
+                        assertThat(GitHubAbuseLimitHandler.DEFAULT_WAIT_MILLIS, equalTo(61 * 1000l));
+                        GitHubAbuseLimitHandler.DEFAULT_WAIT_MILLIS = 3210l;
+                        long waitTime = parseWaitTime(connectorResponse);
+                        assertThat(waitTime, equalTo(GitHubAbuseLimitHandler.DEFAULT_WAIT_MILLIS));
+
                         assertThat(connectorResponse.header("Status"), equalTo("403 Forbidden"));
 
+
                         checkErrorMessageMatches(connectorResponse,
                                 "You have exceeded a secondary rate limit. Please wait a few minutes before you try again");
-                        GitHubAbuseLimitHandler.FAIL.onError(connectorResponse);
+                        GitHubAbuseLimitHandler.WAIT.onError(connectorResponse);
                     }
                 })
                 .build();
 
         gitHub.getMyself();
         assertThat(mockGitHub.getRequestCount(), equalTo(1));
-        try {
-            getTempRepository();
-            fail();
-        } catch (Exception e) {
-            assertThat(e, instanceOf(HttpException.class));
-            assertThat(e.getMessage(), equalTo("Abuse limit reached"));
-        }
-        assertThat(mockGitHub.getRequestCount(), equalTo(2));
+
+        getTempRepository();
+        assertThat(mockGitHub.getRequestCount(), equalTo(3));    
     }
 
     /**
@@ -443,31 +435,24 @@ public void onError(@NotNull GitHubConnectorResponse connectorResponse) throws I
                                         "/repos/hub4j-test-org/temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests"));
                         assertThat(connectorResponse.allHeaders(), instanceOf(Map.class));
                         assertThat(connectorResponse.header("Status"), equalTo("429 Too Many Requests"));
-                        assertThat(connectorResponse.header("Retry-After"), equalTo("42"));
+                        assertThat(connectorResponse.header("Retry-After"), equalTo("8"));
 
                         checkErrorMessageMatches(connectorResponse,
                                 "You have exceeded a secondary rate limit. Please wait a few minutes before you try again");
-                        // Because we've overridden onError to bypass the wait, we don't cover the wait calculation
-                        // logic
-                        // Manually invoke it to make sure it's what we intended
+
                         long waitTime = parseWaitTime(connectorResponse);
-                        assertThat(waitTime, equalTo(42 * 1000l));
+                        assertThat(waitTime, equalTo(8 * 1000l));
 
-                        GitHubAbuseLimitHandler.FAIL.onError(connectorResponse);
+                        GitHubAbuseLimitHandler.WAIT.onError(connectorResponse);
                     }
                 })
                 .build();
 
         gitHub.getMyself();
         assertThat(mockGitHub.getRequestCount(), equalTo(1));
-        try {
-            getTempRepository();
-            fail();
-        } catch (Exception e) {
-            assertThat(e, instanceOf(HttpException.class));
-            assertThat(e.getMessage(), equalTo("Abuse limit reached"));
-        }
-        assertThat(mockGitHub.getRequestCount(), equalTo(2));
+
+        getTempRepository();
+        assertThat(mockGitHub.getRequestCount(), equalTo(3));    
     }
 
     /**
@@ -495,33 +480,26 @@ public void onError(@NotNull GitHubConnectorResponse connectorResponse) throws I
                                 endsWith(
                                         "/repos/hub4j-test-org/temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After"));
                         assertThat(connectorResponse.header("Status"), equalTo("429 Too Many Requests"));
-                        assertThat(connectorResponse.header("Retry-After"), startsWith("Mon"));
+                        assertThat(connectorResponse.header("Retry-After"), containsString("GMT"));
 
                         checkErrorMessageMatches(connectorResponse,
                                 "You have exceeded a secondary rate limit. Please wait a few minutes before you try again");
 
-                        // Because we've overridden onError to bypass the wait, we don't cover the wait calculation
-                        // logic
-                        // Manually invoke it to make sure it's what we intended
                         long waitTime = parseWaitTime(connectorResponse);
-                        // The exact value here will depend on when the test is run, but it should be positive, and huge
-                        assertThat(waitTime, greaterThan(1000 * 1000l));
+                        // The exact value here will depend on when the test is run
+                        assertThat(waitTime, Matchers.lessThan(GitHubAbuseLimitHandler.DEFAULT_WAIT_MILLIS));
+                        assertThat(waitTime, Matchers.greaterThan(3 * 1000l));
 
-                        GitHubAbuseLimitHandler.FAIL.onError(connectorResponse);
+                        GitHubAbuseLimitHandler.WAIT.onError(connectorResponse);
                     }
                 })
                 .build();
 
         gitHub.getMyself();
         assertThat(mockGitHub.getRequestCount(), equalTo(1));
-        try {
-            getTempRepository();
-            fail();
-        } catch (Exception e) {
-            assertThat(e, instanceOf(HttpException.class));
-            assertThat(e.getMessage(), equalTo("Abuse limit reached"));
-        }
-        assertThat(mockGitHub.getRequestCount(), equalTo(2));
+
+        getTempRepository();
+        assertThat(mockGitHub.getRequestCount(), equalTo(3));    
     }
 
     /**
@@ -557,13 +535,11 @@ public void onError(@NotNull GitHubConnectorResponse connectorResponse) throws I
                         checkErrorMessageMatches(connectorResponse,
                                 "You have exceeded a secondary rate limit. Please wait a few minutes before you try again");
 
-                        // Because we've overridden onError to bypass the wait, we don't cover the wait calculation
-                        // logic
-                        // Manually invoke it to make sure it's what we intended
+                        GitHubAbuseLimitHandler.DEFAULT_WAIT_MILLIS = 3210l;
                         long waitTime = parseWaitTime(connectorResponse);
-                        assertThat(waitTime, greaterThan(60000l));
-
-                        GitHubAbuseLimitHandler.FAIL.onError(connectorResponse);
+                        assertThat(waitTime, equalTo(GitHubAbuseLimitHandler.DEFAULT_WAIT_MILLIS));
+        
+                        GitHubAbuseLimitHandler.WAIT.onError(connectorResponse);
                     }
                 })
                 .build();

diff --git a/src/test/java/org/kohsuke/github/GitHubStaticTest.java b/src/test/java/org/kohsuke/github/GitHubStaticTest.java
@@ -416,8 +416,8 @@ public void testGitHubRequest_getApiURL() throws Exception {
         assertThat(e.getCause().getMessage(), equalTo("unknown protocol: gopher"));
 
         e = Assert.assertThrows(GHException.class, () -> GitHubRequest.getApiURL("bogus", "/endpoint"));
-        assertThat(e.getCause(), instanceOf(MalformedURLException.class));
-        assertThat(e.getCause().getMessage(), equalTo("no protocol: bogus/endpoint"));
+        assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
+        assertThat(e.getCause().getMessage(), equalTo("URI is not absolute"));
 
         e = Assert.assertThrows(GHException.class,
                 () -> GitHubRequest.getApiURL(null, "gopher://api.test.github.com/endpoint"));

diff --git a/...t/wiremock/testHandler_Wait_Secondary_Limits_Too_Many_Requests/mappings/2-r_h_t_fail.json b/...t/wiremock/testHandler_Wait_Secondary_Limits_Too_Many_Requests/mappings/2-r_h_t_fail.json
@@ -11,17 +11,14 @@
     }
   },
   "response": {
-    "status": 403,
+    "status": 429,
     "body": "{\"message\":\"You have exceeded a secondary rate limit. Please wait a few minutes before you try again\"}",
     "headers": {
       "Date": "{{now timezone='GMT' format='EEE, dd MMM yyyy HH:mm:ss z'}}",
       "Content-Type": "application/json; charset=utf-8",
       "Server": "GitHub.com",
-      "Status": "403 Forbidden",
-      "gh-limited-by": "search-elapsed-time-shared-grouped",
-      "X-RateLimit-Limit": "5000",
-      "X-RateLimit-Remaining": "4000",
-      "X-RateLimit-Reset": "{{testStartDate offset='3 seconds' format='unix'}}",
+      "Status": "429 Too Many Requests",
+      "Retry-After": "8",
       "Cache-Control": "private, max-age=60, s-maxage=60",
       "Vary": [
         "Accept, Authorization, Cookie, X-GitHub-OTP",
@@ -43,10 +40,10 @@
       "X-GitHub-Request-Id": "CC37:2605:3F982:4E949:5E3C5BFC"
     }
   },
-  "uuid": "79fb1092-8bf3-4274-bc8e-ca126c9d9261",
+  "uuid": "19fb1092-8bf3-4274-bc8e-ca126c9d9261",
   "persistent": true,
-  "scenarioName": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests",
+  "scenarioName": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits1",
   "requiredScenarioState": "Started",
-  "newScenarioState": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests-2",
+  "newScenarioState": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits1-2",
   "insertionIndex": 2
 }
diff --git a/...t/wiremock/testHandler_Wait_Secondary_Limits_Too_Many_Requests/mappings/3-r_h_t_fail.json b/...t/wiremock/testHandler_Wait_Secondary_Limits_Too_Many_Requests/mappings/3-r_h_t_fail.json
@@ -1,8 +1,8 @@
 {
-  "id": "79fb1092-8bf3-4274-bc8e-ca126c9d9261",
-  "name": "repos_hub4j-test-org_temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests",
+  "id": "574da117-6845-46d8-b2c1-4415546ca670",
+  "name": "repos_hub4j-test-org_temp-testHandler_Wait_Secondary_Limits",
   "request": {
-    "url": "/repos/hub4j-test-org/temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests",
+    "url": "/repos/hub4j-test-org/temp-testHandler_Wait_Secondary_Limits",
     "method": "GET",
     "headers": {
       "Accept": {
@@ -11,21 +11,23 @@
     }
   },
   "response": {
-    "status": 429,
-    "body": "{\"message\":\"You have exceeded a secondary rate limit. Please wait a few minutes before you try again\"}",
+    "status": 200,
+    "bodyFileName": "3-r_h_t_fail.json",
     "headers": {
       "Date": "{{now timezone='GMT' format='EEE, dd MMM yyyy HH:mm:ss z'}}",
       "Content-Type": "application/json; charset=utf-8",
       "Server": "GitHub.com",
-      "Status": "429 Too Many Requests",
-      "Retry-After": "42",
+      "Status": "200 OK",
+      "X-RateLimit-Limit": "5000",
+      "X-RateLimit-Remaining": "4922",
+      "X-RateLimit-Reset": "{{testStartDate offset='3 seconds' format='unix'}}",
       "Cache-Control": "private, max-age=60, s-maxage=60",
       "Vary": [
         "Accept, Authorization, Cookie, X-GitHub-OTP",
         "Accept-Encoding"
       ],
-      "ETag": "W/\"7ff3c96399f7ddf6129622d675ca9935\"",
-      "Last-Modified": "Thu, 06 Feb 2020 18:33:37 GMT",
+      "ETag": "W/\"858224998ac7d1fd6dcd43f73d375297\"",
+      "Last-Modified": "Thu, 06 Feb 2020 18:33:43 GMT",
       "X-OAuth-Scopes": "admin:org, admin:org_hook, admin:public_key, admin:repo_hook, delete_repo, gist, notifications, repo, user, write:discussion",
       "X-Accepted-OAuth-Scopes": "repo",
       "X-GitHub-Media-Type": "unknown, github.v3",
@@ -37,13 +39,12 @@
       "X-XSS-Protection": "1; mode=block",
       "Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin",
       "Content-Security-Policy": "default-src 'none'",
-      "X-GitHub-Request-Id": "CC37:2605:3F982:4E949:5E3C5BFC"
+      "X-GitHub-Request-Id": "CC37:2605:3FADC:4EA8C:5E3C5C02"
     }
   },
-  "uuid": "79fb1092-8bf3-4274-bc8e-ca126c9d9261",
+  "uuid": "174da117-6845-46d8-b2c1-4415546ca670",
   "persistent": true,
-  "scenarioName": "scenario-4-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests",
-  "requiredScenarioState": "Started",
-  "newScenarioState": "scenario-4-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests-2",
-  "insertionIndex": 2
+  "scenarioName": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits1",
+  "requiredScenarioState": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits1-2",
+  "insertionIndex": 3
 }
diff --git a/...ndler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After/mappings/2-r_h_t_fail.json b/...ndler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After/mappings/2-r_h_t_fail.json
@@ -11,17 +11,14 @@
     }
   },
   "response": {
-    "status": 403,
+    "status": 429,
     "body": "{\"message\":\"You have exceeded a secondary rate limit. Please wait a few minutes before you try again\"}",
     "headers": {
       "Date": "{{now timezone='GMT' format='EEE, dd MMM yyyy HH:mm:ss z'}}",
       "Content-Type": "application/json; charset=utf-8",
       "Server": "GitHub.com",
-      "Status": "403 Forbidden",
-      "gh-limited-by": "search-elapsed-time-shared-grouped",
-      "X-RateLimit-Limit": "5000",
-      "X-RateLimit-Remaining": "4000",
-      "X-RateLimit-Reset": "{{testStartDate offset='3 seconds' format='unix'}}",
+      "Status": "429 Too Many Requests",
+      "Retry-After": "{{now offset='8 seconds' timezone='GMT' format='EEE, dd MMM yyyy HH:mm:ss z'}}",
       "Cache-Control": "private, max-age=60, s-maxage=60",
       "Vary": [
         "Accept, Authorization, Cookie, X-GitHub-OTP",
@@ -43,10 +40,10 @@
       "X-GitHub-Request-Id": "CC37:2605:3F982:4E949:5E3C5BFC"
     }
   },
-  "uuid": "79fb1092-8bf3-4274-bc8e-ca126c9d9261",
+  "uuid": "22fb1092-8bf3-4274-bc8e-ca126c9d9261",
   "persistent": true,
-  "scenarioName": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After",
+  "scenarioName": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits2",
   "requiredScenarioState": "Started",
-  "newScenarioState": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After-2",
+  "newScenarioState": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits2-2",
   "insertionIndex": 2
 }
diff --git a/...ndler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After/mappings/3-r_h_t_fail.json b/...ndler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After/mappings/3-r_h_t_fail.json
@@ -1,5 +1,5 @@
 {
-  "id": "79fb1092-8bf3-4274-bc8e-ca126c9d9261",
+  "id": "574da117-6845-46d8-b2c1-4415546ca670",
   "name": "repos_hub4j-test-org_temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After",
   "request": {
     "url": "/repos/hub4j-test-org/temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After",
@@ -11,21 +11,23 @@
     }
   },
   "response": {
-    "status": 429,
-    "body": "{\"message\":\"You have exceeded a secondary rate limit. Please wait a few minutes before you try again\"}",
+    "status": 200,
+    "bodyFileName": "3-r_h_t_fail.json",
     "headers": {
       "Date": "{{now timezone='GMT' format='EEE, dd MMM yyyy HH:mm:ss z'}}",
       "Content-Type": "application/json; charset=utf-8",
       "Server": "GitHub.com",
-      "Status": "429 Too Many Requests",
-      "Retry-After": "Mon, 21 Oct 2115 07:28:00 GMT",
+      "Status": "200 OK",
+      "X-RateLimit-Limit": "5000",
+      "X-RateLimit-Remaining": "4922",
+      "X-RateLimit-Reset": "{{testStartDate offset='3 seconds' format='unix'}}",
       "Cache-Control": "private, max-age=60, s-maxage=60",
       "Vary": [
         "Accept, Authorization, Cookie, X-GitHub-OTP",
         "Accept-Encoding"
       ],
-      "ETag": "W/\"7ff3c96399f7ddf6129622d675ca9935\"",
-      "Last-Modified": "Thu, 06 Feb 2020 18:33:37 GMT",
+      "ETag": "W/\"858224998ac7d1fd6dcd43f73d375297\"",
+      "Last-Modified": "Thu, 06 Feb 2020 18:33:43 GMT",
       "X-OAuth-Scopes": "admin:org, admin:org_hook, admin:public_key, admin:repo_hook, delete_repo, gist, notifications, repo, user, write:discussion",
       "X-Accepted-OAuth-Scopes": "repo",
       "X-GitHub-Media-Type": "unknown, github.v3",
@@ -37,13 +39,12 @@
       "X-XSS-Protection": "1; mode=block",
       "Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin",
       "Content-Security-Policy": "default-src 'none'",
-      "X-GitHub-Request-Id": "CC37:2605:3F982:4E949:5E3C5BFC"
+      "X-GitHub-Request-Id": "CC37:2605:3FADC:4EA8C:5E3C5C02"
     }
   },
-  "uuid": "79fb1092-8bf3-4274-bc8e-ca126c9d9261",
+  "uuid": "274da117-6845-46d8-b2c1-4415546ca670",
   "persistent": true,
-  "scenarioName": "scenario-4-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After",
-  "requiredScenarioState": "Started",
-  "newScenarioState": "scenario-4-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits_Too_Many_Requests_Date_Retry_After-2",
-  "insertionIndex": 2
+  "scenarioName": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits2",
+  "requiredScenarioState": "scenario-1-repos-hub4j-test-org-temp-testHandler_Wait_Secondary_Limits2-2",
+  "insertionIndex": 3
 }