commit bbe7c3fc88c64e782bb06769346b61bb7fc18109
author David Pursehouse <dpursehouse@collab.net> Thu Sep 26 11:21:09 2019 +0900
committer David Pursehouse <dpursehouse@collab.net> Thu Sep 26 07:29:40 2019 +0000
tree 9d1b6eef0d59bf58e7e6d923d6ecf5a69993d20a
parent 49d56d43feb80c5d05ed6f8b4f32d54d466d1b95

MultipleQuotaPluginsIT: Clean up and fix tests

The refillsOnException test is intended to verify that when there are
multiple plugins, and an exception is thrown by one of them causing the
operation to fail, quota previously deducted by the ones that succeeded
should be refilled.

The test was instead throwing the exception from the first plugin, which
means no plugins were actually deducting. It was expecting the refill on
the second one, but not verifying that the call was made. Adding the call
to verify() causes the test to fail, because no refill gets called.

Rework the test so that the successfull deduction is done first, and the
second one fails with an exception. Verify that the refill is called on
the first one that was previously successfull.

Also add missing calls to verify on other tests, thus confirming that
the expected calls are actually occurring.

Change-Id: I2cb478d0f64ad1cf66b81a051b126e1f87bcc7c7

javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java

diff --git a/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java b/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java
index 0ad2010..46d11c4 100644
--- a/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java
@@ -87,15 +87,18 @@
         .isEqualTo(
             QuotaResponse.Aggregated.create(
                 ImmutableList.of(QuotaResponse.ok(), QuotaResponse.error("fail"))));
+
+    verify(quotaEnforcerA);
+    verify(quotaEnforcerB);
   }
 
   @Test
   public void refillsOnException() {
     NullPointerException exception = new NullPointerException();
     QuotaRequestContext ctx = QuotaRequestContext.builder().user(identifiedAdmin).build();
-    expect(quotaEnforcerA.requestTokens("testGroup", ctx, 1)).andThrow(exception);
-    expect(quotaEnforcerB.requestTokens("testGroup", ctx, 1)).andReturn(QuotaResponse.ok());
-    quotaEnforcerB.refill("testGroup", ctx, 1);
+    expect(quotaEnforcerA.requestTokens("testGroup", ctx, 1)).andReturn(QuotaResponse.ok());
+    expect(quotaEnforcerB.requestTokens("testGroup", ctx, 1)).andThrow(exception);
+    quotaEnforcerA.refill("testGroup", ctx, 1);
     expectLastCall();
 
     replay(quotaEnforcerA);
@@ -108,6 +111,7 @@
     assertThat(thrown).isEqualTo(exception);
 
     verify(quotaEnforcerA);
+    verify(quotaEnforcerB);
   }
 
   @Test
@@ -124,6 +128,9 @@
         .isEqualTo(
             QuotaResponse.Aggregated.create(
                 ImmutableList.of(QuotaResponse.error("fail"), QuotaResponse.noOp())));
+
+    verify(quotaEnforcerA);
+    verify(quotaEnforcerB);
   }
 
   @Test
@@ -139,6 +146,9 @@
         quotaBackend.user(identifiedAdmin).availableTokens("testGroup").availableTokens();
     assertThat(tokens).isPresent();
     assertThat(tokens.getAsLong()).isEqualTo(10L);
+
+    verify(quotaEnforcerA);
+    verify(quotaEnforcerB);
   }
 
   @Test
@@ -154,5 +164,8 @@
         quotaBackend.user(identifiedAdmin).availableTokens("testGroup").availableTokens();
     assertThat(tokens).isPresent();
     assertThat(tokens.getAsLong()).isEqualTo(20L);
+
+    verify(quotaEnforcerA);
+    verify(quotaEnforcerB);
   }
 }