From 424247f4a4a34bc5c180406e5d8f457bca1a6cfc Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 16 Jul 2020 16:59:12 -0700 Subject: [PATCH] Use github-api v1.115 --- pom.xml | 4 +- .../ApiRateLimitCheckerTest.java | 74 ++++++++++++++----- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/pom.xml b/pom.xml index 07157e7bd..57e079900 100644 --- a/pom.xml +++ b/pom.xml @@ -46,7 +46,7 @@ org.jenkins-ci.plugins github-api - 1.111.4 + 1.115 @@ -57,7 +57,7 @@ com.coravy.hudson.plugins.github github - 1.29.3 + 1.31.0 io.jsonwebtoken diff --git a/src/test/java/org/jenkinsci/plugins/github_branch_source/ApiRateLimitCheckerTest.java b/src/test/java/org/jenkinsci/plugins/github_branch_source/ApiRateLimitCheckerTest.java index 5c0132b76..9465e3851 100644 --- a/src/test/java/org/jenkinsci/plugins/github_branch_source/ApiRateLimitCheckerTest.java +++ b/src/test/java/org/jenkinsci/plugins/github_branch_source/ApiRateLimitCheckerTest.java @@ -1,6 +1,8 @@ package org.jenkinsci.plugins.github_branch_source; +import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.ScenarioMappingBuilder; +import com.github.tomakehurst.wiremock.matching.RequestPatternBuilder; import com.github.tomakehurst.wiremock.stubbing.Scenario; import hudson.util.LogTaskListener; import hudson.util.RingBufferLogHandler; @@ -46,6 +48,10 @@ private long countOfOutputLinesContaining(String substring) { return countOfOutputLines(m -> m.contains(substring)); } + public static int getRequestCount(WireMockServer server) { + return server.countRequestsMatching(RequestPatternBuilder.allRequests().build()).getCount(); + } + private class RateLimit { final int remaining; final int limit; @@ -136,6 +142,7 @@ public void ThrottleOnOverTestWithQuota() throws Exception { } assertEquals(0, countOfOutputLinesContaining("Sleeping")); + assertEquals(100, getRequestCount(githubApi)); } /** @@ -158,6 +165,7 @@ public void ThrottleOnNormalizeTestWithQuota() throws Exception { } assertEquals(0, countOfOutputLinesContaining("Sleeping")); + assertEquals(100, getRequestCount(githubApi)); } /** @@ -180,7 +188,7 @@ public void ThrottleOnOverTest() throws Exception { } // finally, stop throttling by restoring quota - scenarios.add(new RateLimit(limit, limit, soon)); + scenarios.add(new RateLimit(limit, limit, new Date(soon.getTime() + 2000))); setupStubs(scenarios); // check rate limit to hit the first 11 scenarios because the throttle (add more here) @@ -192,6 +200,8 @@ public void ThrottleOnOverTest() throws Exception { //should be no output assertEquals(0, countOfOutputLines(m -> m.matches("[sS]leeping"))); + assertEquals(11, getRequestCount(githubApi)); + // check rate limit to hit the next 10 scenarios ApiRateLimitChecker.ThrottleOnOver.checkApiRateLimit(listener, github); @@ -199,12 +209,14 @@ public void ThrottleOnOverTest() throws Exception { assertEquals(1, countOfOutputLinesContaining("Sleeping for")); assertEquals(expectedNumThrottles-1, countOfOutputLinesContaining("Still sleeping")); assertEquals(1, countOfOutputLinesContaining("refreshed")); + assertEquals(23, getRequestCount(githubApi)); //Make sure no new output ApiRateLimitChecker.ThrottleOnOver.checkApiRateLimit(listener, github); assertEquals(1, countOfOutputLinesContaining("Sleeping for")); assertEquals(expectedNumThrottles-1, countOfOutputLinesContaining("Still sleeping")); assertEquals(1, countOfOutputLinesContaining("refreshed")); + assertEquals(24, getRequestCount(githubApi)); } /** @@ -234,26 +246,30 @@ public void ThrottleForNormalizeTestWithinIdeal() throws Exception { scenarios.add(new RateLimit(limit, approximateIdeal - 101, soon)); // Check that we can back to our original throttle - scenarios.add(new RateLimit(limit, approximateIdeal - 100, soon)); + // ignored as invalid by github-api library + scenarios.add(new RateLimit(limit, approximateIdeal - 100, new Date(soon.getTime() + 2000))); // "Less" under the ideal but should recheck and throttle again - scenarios.add(new RateLimit(limit, approximateIdeal - 99, soon)); + scenarios.add(new RateLimit(limit, approximateIdeal - 99, new Date(soon.getTime() + 2000))); // Check that we are under our ideal so we should throttle - scenarios.add(new RateLimit(limit, approximateIdeal - 99, soon)); - + scenarios.add(new RateLimit(limit, approximateIdeal - 99, new Date(soon.getTime() + 2000))); // Reset back to a full limit - scenarios.add(new RateLimit(limit, limit, soon)); + scenarios.add(new RateLimit(limit, limit, new Date(soon.getTime() + 3000))); setupStubs(scenarios); // First check will say under budget (add counts) ApiRateLimitChecker.ThrottleForNormalize.checkApiRateLimit(listener, github); + + assertEquals(1, getRequestCount(githubApi)); assertEquals(1, countOfOutputLinesContaining("under budget")); assertFalse(handler.getView().stream().anyMatch(m -> m.getMessage().contains("Sleeping"))); // Second check will go over budget ApiRateLimitChecker.ThrottleForNormalize.checkApiRateLimit(listener, github); + + assertEquals(9, getRequestCount(githubApi)); assertEquals(2, countOfOutputLinesContaining("rechecking")); assertEquals(3, countOfOutputLinesContaining("Still sleeping")); assertEquals(2, countOfOutputLinesContaining("Sleeping for")); @@ -285,7 +301,7 @@ public void NormalizeThrottleWithBurnedBuffer() throws Exception { // Trigger a throttle but we have burned our buffer scenarios.add(new RateLimit(limit, 0, soon)); // Refresh rate limit - scenarios.add(new RateLimit(limit, limit, soon)); + scenarios.add(new RateLimit(limit, limit, new Date(soon.getTime() + 2000))); setupStubs(scenarios); // Run check against API limit @@ -303,6 +319,7 @@ public void NormalizeThrottleWithBurnedBuffer() throws Exception { assertEquals(4, countOfOutputLinesContaining("Sleeping")); // Expect that we stopped waiting on a refresh assertEquals(1, countOfOutputLinesContaining("refreshed")); + assertEquals(6, getRequestCount(githubApi)); } /** @@ -329,7 +346,7 @@ public void OnOverThrottleTimingRateLimitCheck() throws Exception { } // finally, stop throttling by restoring quota - scenarios.add(new RateLimit(limit, limit, soon)); + scenarios.add(new RateLimit(limit, limit, new Date(soon.getTime() + 2000))); setupStubs(scenarios); long start = System.currentTimeMillis(); @@ -339,12 +356,16 @@ public void OnOverThrottleTimingRateLimitCheck() throws Exception { ApiRateLimitChecker.ThrottleOnOver.checkApiRateLimit(listener, github); } + assertEquals(6, getRequestCount(githubApi)); + //should be no output assertEquals(0, countOfOutputLinesContaining("Sleeping")); // check rate limit to hit the next 5 scenarios ApiRateLimitChecker.ThrottleOnOver.checkApiRateLimit(listener, github); + assertEquals(13, getRequestCount(githubApi)); + //want to make sure that the 5 API checks (the last one is resetting) are taking at least 1000 MS assertTrue((System.currentTimeMillis() - start) > 1000); @@ -354,6 +375,7 @@ public void OnOverThrottleTimingRateLimitCheck() throws Exception { //no new output ApiRateLimitChecker.ThrottleOnOver.checkApiRateLimit(listener, github); + assertEquals(14, getRequestCount(githubApi)); assertEquals(1, countOfOutputLinesContaining("Sleeping")); assertEquals(expectedNumThrottles - 1, countOfOutputLinesContaining("Still sleeping")); } @@ -383,7 +405,9 @@ public void NormalizeThrottleTimingRateLimitCheck() throws Exception { // Trigger a wait until rate limit scenarios.add(new RateLimit(limit, approximateIdeal - burst, soon)); // Refresh rate limit - scenarios.add(new RateLimit(limit, limit, soon)); + // github-api will ignore ratelimit responses that appear invalid + // Rate limit only goes up when the the reset date is later than previous records. + scenarios.add(new RateLimit(limit, limit, new Date(soon.getTime() + 2000))); setupStubs(scenarios); ApiRateLimitChecker.ThrottleForNormalize.checkApiRateLimit(listener, github); @@ -393,6 +417,8 @@ public void NormalizeThrottleTimingRateLimitCheck() throws Exception { // Run check ApiRateLimitChecker.ThrottleForNormalize.checkApiRateLimit(listener, github); + assertEquals(6, getRequestCount(githubApi)); + // Want to make sure that the 3 API checks are taking at least 600 MS assertTrue((System.currentTimeMillis() - start) > 600); // Expect a triggered throttle for normalize @@ -425,31 +451,31 @@ public void NormalizeExpectedIdealOverTime() throws Exception { // buffer for this limit will be limit/20 = 250 // burst for this will be limit/5 = 1000 // Ideal calculated at 45, 30, 15, and 0 minutes - int[] morePreciseIdeal = {612, 424, 237, 50}; + int[] morePreciseIdeal = {50, 237, 424, 612}; // deadline set for those times as well for (int i = 0; i < 4; i++) { - scenarios.add(new RateLimit(limit, morePreciseIdeal[i], new Date(start + TimeUnit.MINUTES.toMillis((3 - i) * 15)))); + scenarios.add(new RateLimit(limit, morePreciseIdeal[i], new Date(start + TimeUnit.MINUTES.toMillis(i * 15)))); } /* * With the limit at 400: the burst will be limit/10 and buffer will be limit/20 */ limit = 400; - morePreciseIdeal = new int[]{274, 189, 104, 20}; + morePreciseIdeal = new int[]{20, 104, 189, 274}; // deadline set for those times as well for (int i = 0; i < 4; i++) { - scenarios.add(new RateLimit(limit, morePreciseIdeal[i], new Date(start + TimeUnit.MINUTES.toMillis((3 - i) * 15)))); + scenarios.add(new RateLimit(limit, morePreciseIdeal[i], new Date(start + TimeUnit.MINUTES.toMillis(i * 15)))); } /* * With the limit at 1000: the burst will be limit/5 and buffer will be 15 */ limit = 200; - morePreciseIdeal = new int[]{138, 97, 56, 15}; + morePreciseIdeal = new int[]{15, 56, 97, 138}; // deadline set for those times as well for (int i = 0; i < 4; i++) { - scenarios.add(new RateLimit(limit, morePreciseIdeal[i], new Date(start + TimeUnit.MINUTES.toMillis((3 - i) * 15)))); + scenarios.add(new RateLimit(limit, morePreciseIdeal[i], new Date(start + TimeUnit.MINUTES.toMillis(i * 15)))); } setupStubs(scenarios); @@ -458,6 +484,8 @@ public void NormalizeExpectedIdealOverTime() throws Exception { ApiRateLimitChecker.ThrottleForNormalize.checkApiRateLimit(listener, github); } + assertEquals(12, getRequestCount(githubApi)); + // Expect a triggered throttle for normalize assertEquals(12, countOfOutputLinesContaining("Current quota")); // Making sure the budgets are correct @@ -479,12 +507,16 @@ public void OnOverExpectedIdealOverTime() throws Exception { List scenarios = new ArrayList<>(); int limit = 1000; // estimate the ideal(which does not apply in this scenario) - int[] morePreciseIdeal = {612, 424, 237, 49}; + // Rate limit + int[] morePreciseIdeal = {49, 237, 424, 612}; + // Rate limit records that expire early than the last returned are ignored as invalid + // Must be the same or greater // deadline set for those times as well for (int i = 0; i < 4; i++) { - scenarios.add(new RateLimit(limit, morePreciseIdeal[i], new Date(start + TimeUnit.MINUTES.toMillis((3 - i) * 15)))); + scenarios.add(new RateLimit(limit, morePreciseIdeal[i], new Date(start + TimeUnit.MINUTES.toMillis((i) * 15)))); } + // Refresh rate limit scenarios.add(new RateLimit(limit, limit, soon)); setupStubs(scenarios); @@ -494,6 +526,8 @@ public void OnOverExpectedIdealOverTime() throws Exception { ApiRateLimitChecker.ThrottleOnOver.checkApiRateLimit(listener, github); } + assertEquals(6, getRequestCount(githubApi)); + // Expect this to only get throttled when we are over the buffer limit assertEquals(1, countOfOutputLinesContaining("Current quota")); //Making sure the budget messages are correct @@ -516,9 +550,9 @@ public void ExpectedResetTimingNormalize() throws Exception { List scenarios = new ArrayList<>(); int limit = 1000; int buffer = 50; - scenarios.add(new RateLimit(limit, limit, soon)); //Giving a bit of time to make sure the setup happens on time long start = System.currentTimeMillis() + 7000; + scenarios.add(new RateLimit(limit, limit, new Date(start))); for (int i = 0; i <= 3; i++) { scenarios.add(new RateLimit(limit, buffer - 5, new Date(start))); @@ -540,6 +574,7 @@ public void ExpectedResetTimingNormalize() throws Exception { // Expect a triggered throttle for normalize assertEquals(2, countOfOutputLinesContaining("Current quota")); assertEquals(2, countOfOutputLinesContaining("Still sleeping")); + assertEquals(6, getRequestCount(githubApi)); } /** @@ -557,9 +592,9 @@ public void ExpectedResetTimingOnOver() throws Exception { List scenarios = new ArrayList<>(); int limit = 1000; int buffer = 50; - scenarios.add(new RateLimit(limit, limit, soon)); //Giving a bit of time to make sure the setup happens on time long start = System.currentTimeMillis() + 7000; + scenarios.add(new RateLimit(limit, limit, new Date(start))); for (int i = 0; i <= 3; i++) { scenarios.add(new RateLimit(limit, buffer - 5, new Date(start))); @@ -580,5 +615,6 @@ public void ExpectedResetTimingOnOver() throws Exception { // We have 7 "notify" type messages and 2 "expired" type messages assertEquals(2, countOfOutputLinesContaining("Current quota")); assertEquals(2, countOfOutputLinesContaining("Still sleeping")); + assertEquals(6, getRequestCount(githubApi)); } }