opendev/gerrit
Code Issues Proposed changes

Don't return the TestAccount on an account update

Browse Source 
Any test which updates a specific property of an account knows which
part is changed in which way. Depending on the returned account object
and passing it (or its non-ID properties) to further queries and
assertions as expected values is a bad style. Further assertions should
rather explicitly state their expected values. To not encourage the use
of the updated account, we deliberately do not return it from the
update.

Change-Id: I996f6654ccdb3e17bdd4e7205fdba05747ab6b60
This commit is contained in:
Alice Kober-Sotzek
2018-08-24 17:40:53 +02:00
parent 62bea21ab1
commit f48517c73c
3 changed files with 27 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java Normal file
View File

@@ -0,0 +1,20 @@
// Copyright (C) 2018 The Android Open Source Project
//
// 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
//
// http://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.
package com.google.gerrit.acceptance.testsuite;
@FunctionalInterface
public interface ThrowingConsumer<T> {
void accept(T t) throws Exception;
}

3
java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
View File

@@ -138,13 +138,12 @@ public class AccountOperationsImpl implements AccountOperations {
return TestAccountUpdate.builder(this::updateAccount);
}
private TestAccount updateAccount(TestAccountUpdate accountUpdate)
private void updateAccount(TestAccountUpdate accountUpdate)
throws OrmException, IOException, ConfigInvalidException {
AccountsUpdate.AccountUpdater accountUpdater =
(account, updateBuilder) -> fillBuilder(updateBuilder, accountUpdate, accountId);
Optional<AccountState> updatedAccount = updateAccount(accountUpdater);
checkState(updatedAccount.isPresent(), "Tried to update non-existing test account");
return toTestAccount(updatedAccount.get());
}
private Optional<AccountState> updateAccount(AccountsUpdate.AccountUpdater accountUpdater)

13
java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
View File

@@ -15,7 +15,7 @@
package com.google.gerrit.acceptance.testsuite.account;
import com.google.auto.value.AutoValue;
import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
import com.google.gerrit.acceptance.testsuite.ThrowingConsumer;
import java.util.Optional;
@AutoValue
@@ -32,9 +32,9 @@ public abstract class TestAccountUpdate {
public abstract Optional<Boolean> active();
abstract ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater();
abstract ThrowingConsumer<TestAccountUpdate> accountUpdater();
public static Builder builder(ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater) {
public static Builder builder(ThrowingConsumer<TestAccountUpdate> accountUpdater) {
return new AutoValue_TestAccountUpdate.Builder()
.accountUpdater(accountUpdater)
.httpPassword("http-pass");
@@ -82,14 +82,13 @@ public abstract class TestAccountUpdate {
return active(false);
}
abstract Builder accountUpdater(
ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater);
abstract Builder accountUpdater(ThrowingConsumer<TestAccountUpdate> accountUpdater);
abstract TestAccountUpdate autoBuild();
public TestAccount update() throws Exception {
public void update() throws Exception {
TestAccountUpdate accountUpdate = autoBuild();
return accountUpdate.accountUpdater().apply(accountUpdate);
accountUpdate.accountUpdater().accept(accountUpdate);
}
}
}