Merge changes Ic857fc10,Ibc1c742a,If2760542 * changes: ReceiveCommits: Fix error message when Force Push is required PushPermissionsIT: Add tests for remaining checks PushPermissionsIT: Add many more tests
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 444f5b1..cadab83 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt
@@ -3925,6 +3925,15 @@ + Default is 1. +[[execution.fanOutThreadPoolSize]]execution.fanOutThreadPoolSize:: ++ +Maximum size of thread pool to on which a serving thread can fan-out +work to parallelize it. ++ +When set to 0, a direct executor will be used. ++ +By default, 25 which means that formatting happens in the caller thread. + [[receiveemail]] === Section receiveemail
diff --git a/java/com/google/gerrit/server/FanOutExecutor.java b/java/com/google/gerrit/server/FanOutExecutor.java new file mode 100644 index 0000000..a489890 --- /dev/null +++ b/java/com/google/gerrit/server/FanOutExecutor.java
@@ -0,0 +1,27 @@ +// 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.server; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import com.google.inject.BindingAnnotation; +import java.lang.annotation.Retention; + +/** + * Marker on the global {@code ThreadPoolExecutor} used to do parallel work from a serving thread. + */ +@Retention(RUNTIME) +@BindingAnnotation +public @interface FanOutExecutor {}
diff --git a/java/com/google/gerrit/server/account/AccountCache.java b/java/com/google/gerrit/server/account/AccountCache.java index b6ca1cb..17493bf 100644 --- a/java/com/google/gerrit/server/account/AccountCache.java +++ b/java/com/google/gerrit/server/account/AccountCache.java
@@ -16,7 +16,9 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.reviewdb.client.Account; +import java.util.Map; import java.util.Optional; +import java.util.Set; /** Caches important (but small) account state to avoid database hits. */ public interface AccountCache { @@ -31,6 +33,20 @@ Optional<AccountState> get(Account.Id accountId); /** + * Returns a {@code Map} of {@code Account.Id} to {@code AccountState} for the given account IDs. + * If not cached yet the accounts are loaded. If an account can't be loaded (e.g. because it is + * missing), the entry will be missing from the result. + * + * <p>Loads accounts in parallel if applicable. + * + * @param accountIds IDs of the account that should be retrieved + * @return {@code Map} of {@code Account.Id} to {@code AccountState} instances for the given + * account IDs, if an account can't be loaded (e.g. because it is missing), the entry will be + * missing from the result + */ + Map<Account.Id, AccountState> get(Set<Account.Id> accountIds); + + /** * Returns an {@code AccountState} instance for the given account ID. If not cached yet the * account is loaded. Returns an empty {@code AccountState} instance to represent a missing * account.
diff --git a/java/com/google/gerrit/server/account/AccountCacheImpl.java b/java/com/google/gerrit/server/account/AccountCacheImpl.java index 895c5b6..0648f9f 100644 --- a/java/com/google/gerrit/server/account/AccountCacheImpl.java +++ b/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -18,9 +18,11 @@ import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import com.google.common.collect.ImmutableMap; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.server.FanOutExecutor; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIds; import com.google.gerrit.server.cache.CacheModule; @@ -31,8 +33,16 @@ import com.google.inject.TypeLiteral; import com.google.inject.name.Named; import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.Set; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; import org.eclipse.jgit.errors.ConfigInvalidException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,15 +70,18 @@ private final AllUsersName allUsersName; private final ExternalIds externalIds; private final LoadingCache<Account.Id, Optional<AccountState>> byId; + private final ExecutorService executor; @Inject AccountCacheImpl( AllUsersName allUsersName, ExternalIds externalIds, - @Named(BYID_NAME) LoadingCache<Account.Id, Optional<AccountState>> byId) { + @Named(BYID_NAME) LoadingCache<Account.Id, Optional<AccountState>> byId, + @FanOutExecutor ExecutorService executor) { this.allUsersName = allUsersName; this.externalIds = externalIds; this.byId = byId; + this.executor = executor; } @Override @@ -92,6 +105,41 @@ } @Override + public Map<Account.Id, AccountState> get(Set<Account.Id> accountIds) { + Map<Account.Id, AccountState> accountStates = new HashMap<>(accountIds.size()); + List<Callable<Optional<AccountState>>> callables = new ArrayList<>(); + for (Account.Id accountId : accountIds) { + Optional<AccountState> state = byId.getIfPresent(accountId); + if (state != null) { + // The value is in-memory, so we just get the state + state.ifPresent(s -> accountStates.put(accountId, s)); + } else { + // Queue up a callable so that we can load accounts in parallel + callables.add(() -> get(accountId)); + } + } + if (callables.isEmpty()) { + return accountStates; + } + + List<Future<Optional<AccountState>>> futures; + try { + futures = executor.invokeAll(callables); + } catch (InterruptedException e) { + log.error("Cannot load AccountStates", e); + return ImmutableMap.of(); + } + for (Future<Optional<AccountState>> f : futures) { + try { + f.get().ifPresent(s -> accountStates.put(s.getAccount().getId(), s)); + } catch (InterruptedException | ExecutionException e) { + log.error("Cannot load AccountState", e); + } + } + return accountStates; + } + + @Override public Optional<AccountState> getByUsername(String username) { try { return externalIds
diff --git a/java/com/google/gerrit/server/account/InternalAccountDirectory.java b/java/com/google/gerrit/server/account/InternalAccountDirectory.java index 77752da..8c2bc10 100644 --- a/java/com/google/gerrit/server/account/InternalAccountDirectory.java +++ b/java/com/google/gerrit/server/account/InternalAccountDirectory.java
@@ -15,8 +15,10 @@ package com.google.gerrit.server.account; import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; import com.google.common.base.Strings; +import com.google.common.collect.Streams; import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.common.AvatarInfo; import com.google.gerrit.extensions.registration.DynamicItem; @@ -32,7 +34,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.List; -import java.util.Optional; +import java.util.Map; import java.util.Set; @Singleton @@ -66,11 +68,14 @@ if (options.equals(ID_ONLY)) { return; } + Set<Account.Id> ids = + Streams.stream(in).map(a -> new Account.Id(a._accountId)).collect(toSet()); + Map<Account.Id, AccountState> accountStates = accountCache.get(ids); for (AccountInfo info : in) { Account.Id id = new Account.Id(info._accountId); - Optional<AccountState> state = accountCache.get(id); - if (state.isPresent()) { - fill(info, state.get(), options); + AccountState state = accountStates.get(id); + if (state != null) { + fill(info, accountStates.get(id), options); } else { info._accountId = options.contains(FillOptions.ID) ? id.get() : null; }
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java index c27c09a..82affe0 100644 --- a/java/com/google/gerrit/server/change/ChangeJson.java +++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -44,7 +44,6 @@ import com.google.common.base.Joiner; import com.google.common.base.MoreObjects; import com.google.common.base.Throwables; -import com.google.common.collect.FluentIterable; import com.google.common.collect.HashBasedTable; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -106,6 +105,7 @@ import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.FanOutExecutor; import com.google.gerrit.server.GpgException; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.ReviewerByEmailSet; @@ -129,7 +129,6 @@ import com.google.gerrit.server.permissions.LabelPermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; -import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.RemoveReviewerControl; @@ -155,6 +154,10 @@ import java.util.Optional; import java.util.Set; import java.util.TreeMap; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -271,6 +274,8 @@ private final RemoveReviewerControl removeReviewerControl; private final TrackingFooters trackingFooters; private final Metrics metrics; + private final ExecutorService fanOutExecutor; + private boolean lazyLoad = true; private AccountLoader accountLoader; private FixInput fix; @@ -304,6 +309,7 @@ RemoveReviewerControl removeReviewerControl, TrackingFooters trackingFooters, Metrics metrics, + @FanOutExecutor ExecutorService fanOutExecutor, @Assisted Iterable<ListChangesOption> options) { this.db = db; this.userProvider = user; @@ -331,6 +337,7 @@ this.removeReviewerControl = removeReviewerControl; this.trackingFooters = trackingFooters; this.metrics = metrics; + this.fanOutExecutor = fanOutExecutor; this.options = Sets.immutableEnumSet(options); } @@ -411,12 +418,11 @@ throws OrmException { try (Timer0.Context ignored = metrics.formatQueryResultsLatency.start()) { accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); - ensureLoaded(FluentIterable.from(in).transformAndConcat(QueryResult::entities)); - - List<List<ChangeInfo>> res = Lists.newArrayListWithCapacity(in.size()); - Map<Change.Id, ChangeInfo> out = new HashMap<>(); + List<List<ChangeInfo>> res = new ArrayList<>(in.size()); + Map<Change.Id, ChangeInfo> cache = Maps.newHashMapWithExpectedSize(in.size()); for (QueryResult<ChangeData> r : in) { - List<ChangeInfo> infos = toChangeInfos(out, r.entities()); + List<ChangeInfo> infos = toChangeInfos(r.entities(), cache); + infos.forEach(c -> cache.put(new Change.Id(c._number), c)); if (!infos.isEmpty() && r.more()) { infos.get(infos.size() - 1)._moreChanges = true; } @@ -478,38 +484,53 @@ return options.contains(option); } - private List<ChangeInfo> toChangeInfos(Map<Change.Id, ChangeInfo> out, List<ChangeData> changes) { + private List<ChangeInfo> toChangeInfos( + List<ChangeData> changes, Map<Change.Id, ChangeInfo> cache) { try (Timer0.Context ignored = metrics.toChangeInfosLatency.start()) { - List<ChangeInfo> info = Lists.newArrayListWithCapacity(changes.size()); + // Create a list of formatting calls that can be called sequentially or in parallel + List<Callable<Optional<ChangeInfo>>> formattingCalls = new ArrayList<>(changes.size()); for (ChangeData cd : changes) { - ChangeInfo i = out.get(cd.getId()); - if (i == null) { - try { - i = toChangeInfo(cd, Optional.empty()); - } catch (PatchListNotAvailableException - | GpgException - | OrmException - | IOException - | PermissionBackendException - | RuntimeException e) { - if (has(CHECK)) { - i = checkOnly(cd); - } else if (e instanceof NoSuchChangeException) { - log.info( - "NoSuchChangeException: Omitting corrupt change " - + cd.getId() - + " from results. Seems to be stale in the index."); - continue; - } else { - log.warn("Omitting corrupt change " + cd.getId() + " from results", e); - continue; - } - } - out.put(cd.getId(), i); - } - info.add(i); + formattingCalls.add( + () -> { + ChangeInfo i = cache.get(cd.getId()); + if (i != null) { + return Optional.of(i); + } + try { + ensureLoaded(Collections.singleton(cd)); + return Optional.of(format(cd, Optional.empty(), false)); + } catch (OrmException | RuntimeException e) { + log.warn("Omitting corrupt change " + cd.getId() + " from results", e); + return Optional.empty(); + } + }); } - return info; + + long numProjects = changes.stream().map(c -> c.project()).distinct().count(); + if (!lazyLoad || changes.size() < 3 || numProjects < 2) { + // Format these changes in the request thread as the multithreading overhead would be too + // high. + List<ChangeInfo> result = new ArrayList<>(changes.size()); + for (Callable<Optional<ChangeInfo>> c : formattingCalls) { + try { + c.call().ifPresent(result::add); + } catch (Exception e) { + log.warn("Omitting change due to exception", e); + } + } + return result; + } + + // Format the changes in parallel on the executor + List<ChangeInfo> result = new ArrayList<>(changes.size()); + try { + for (Future<Optional<ChangeInfo>> f : fanOutExecutor.invokeAll(formattingCalls)) { + f.get().ifPresent(result::add); + } + } catch (InterruptedException | ExecutionException e) { + throw new IllegalStateException(e); + } + return result; } }
diff --git a/java/com/google/gerrit/server/config/SysExecutorModule.java b/java/com/google/gerrit/server/config/SysExecutorModule.java index 99edb65..b9fe34c 100644 --- a/java/com/google/gerrit/server/config/SysExecutorModule.java +++ b/java/com/google/gerrit/server/config/SysExecutorModule.java
@@ -17,6 +17,7 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.ThreadFactoryBuilder; +import com.google.gerrit.server.FanOutExecutor; import com.google.gerrit.server.git.WorkQueue; import com.google.inject.AbstractModule; import com.google.inject.Provides; @@ -63,6 +64,17 @@ @Provides @Singleton + @FanOutExecutor + public ExecutorService createFanOutExecutor(@GerritServerConfig Config config, WorkQueue queues) { + int poolSize = config.getInt("execution", null, "fanOutThreadPoolSize", 25); + if (poolSize == 0) { + return MoreExecutors.newDirectExecutorService(); + } + return queues.createQueue(poolSize, "FanOut"); + } + + @Provides + @Singleton @ChangeUpdateExecutor public ListeningExecutorService createChangeUpdateExecutor(@GerritServerConfig Config config) { int poolSize = config.getInt("receive", null, "changeUpdateThreads", 1);
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java index 78687cd..dae37d6 100644 --- a/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java +++ b/java/com/google/gerrit/server/restapi/change/ReviewerRecommender.java
@@ -29,10 +29,10 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ApprovalsUtil; +import com.google.gerrit.server.FanOutExecutor; import com.google.gerrit.server.change.ReviewerSuggestion; import com.google.gerrit.server.change.SuggestedReviewer; import com.google.gerrit.server.config.GerritServerConfig; -import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.index.change.ChangeField; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ProjectState; @@ -54,6 +54,7 @@ import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; @@ -78,7 +79,7 @@ private final Config config; private final DynamicMap<ReviewerSuggestion> reviewerSuggestionPluginMap; private final Provider<InternalChangeQuery> queryProvider; - private final WorkQueue workQueue; + private final ExecutorService executor; private final Provider<ReviewDb> dbProvider; private final ApprovalsUtil approvalsUtil; @@ -87,7 +88,7 @@ ChangeQueryBuilder changeQueryBuilder, DynamicMap<ReviewerSuggestion> reviewerSuggestionPluginMap, Provider<InternalChangeQuery> queryProvider, - WorkQueue workQueue, + @FanOutExecutor ExecutorService executor, Provider<ReviewDb> dbProvider, ApprovalsUtil approvalsUtil, @GerritServerConfig Config config) { @@ -95,7 +96,7 @@ this.config = config; this.queryProvider = queryProvider; this.reviewerSuggestionPluginMap = reviewerSuggestionPluginMap; - this.workQueue = workQueue; + this.executor = executor; this.dbProvider = dbProvider; this.approvalsUtil = approvalsUtil; } @@ -150,7 +151,7 @@ try { List<Future<Set<SuggestedReviewer>>> futures = - workQueue.getDefaultQueue().invokeAll(tasks, PLUGIN_QUERY_TIMEOUT, TimeUnit.MILLISECONDS); + executor.invokeAll(tasks, PLUGIN_QUERY_TIMEOUT, TimeUnit.MILLISECONDS); Iterator<Double> weightIterator = weights.iterator(); for (Future<Set<SuggestedReviewer>> f : futures) { double weight = weightIterator.next();
diff --git a/java/com/google/gerrit/testing/FakeAccountCache.java b/java/com/google/gerrit/testing/FakeAccountCache.java index e549e08..224a5bf 100644 --- a/java/com/google/gerrit/testing/FakeAccountCache.java +++ b/java/com/google/gerrit/testing/FakeAccountCache.java
@@ -14,6 +14,8 @@ package com.google.gerrit.testing; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.reviewdb.client.Account; @@ -24,6 +26,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.Set; /** Fake implementation of {@link AccountCache} for testing. */ public class FakeAccountCache implements AccountCache { @@ -48,6 +51,11 @@ } @Override + public synchronized Map<Account.Id, AccountState> get(Set<Account.Id> accountIds) { + return ImmutableMap.copyOf(Maps.filterKeys(byId, accountIds::contains)); + } + + @Override public synchronized Optional<AccountState> getByUsername(String username) { throw new UnsupportedOperationException(); }
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java index 6d90510..b63830e 100644 --- a/java/com/google/gerrit/testing/InMemoryModule.java +++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -29,6 +29,7 @@ import com.google.gerrit.metrics.DisabledMetricMaker; import com.google.gerrit.metrics.MetricMaker; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.FanOutExecutor; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdentProvider; import com.google.gerrit.server.api.GerritApiModule; @@ -273,6 +274,13 @@ @Provides @Singleton + @FanOutExecutor + public ExecutorService createChangeJsonExecutor() { + return MoreExecutors.newDirectExecutorService(); + } + + @Provides + @Singleton @GerritServerId public String createServerId() { String serverId =
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java index 545c2a5..6cfc583 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticQueryAccountsTest.java
@@ -23,6 +23,7 @@ import com.google.inject.Injector; import java.util.concurrent.ExecutionException; import org.eclipse.jgit.lib.Config; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -52,11 +53,22 @@ } } + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @After + public void cleanupIndex() { + if (nodeInfo != null) { + ElasticTestUtils.deleteAllIndexes(nodeInfo, testName()); + } + } + @Override protected Injector createInjector() { Config elasticsearchConfig = new Config(config); InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getMethodName().toLowerCase() + "_"; + String indicesPrefix = testName(); ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix); return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java index d375a0c..5949c06 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticQueryChangesTest.java
@@ -25,6 +25,7 @@ import java.util.concurrent.ExecutionException; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Config; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -55,11 +56,22 @@ } } + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @After + public void cleanupIndex() { + if (nodeInfo != null) { + ElasticTestUtils.deleteAllIndexes(nodeInfo, testName()); + } + } + @Override protected Injector createInjector() { Config elasticsearchConfig = new Config(config); InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getMethodName().toLowerCase() + "_"; + String indicesPrefix = testName(); ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix); return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticQueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticQueryGroupsTest.java index 82cedc0..a1c331d 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticQueryGroupsTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticQueryGroupsTest.java
@@ -23,6 +23,7 @@ import com.google.inject.Injector; import java.util.concurrent.ExecutionException; import org.eclipse.jgit.lib.Config; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -52,11 +53,22 @@ } } + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @After + public void cleanupIndex() { + if (nodeInfo != null) { + ElasticTestUtils.deleteAllIndexes(nodeInfo, testName()); + } + } + @Override protected Injector createInjector() { Config elasticsearchConfig = new Config(config); InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getMethodName().toLowerCase() + "_"; + String indicesPrefix = testName(); ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix); return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticQueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticQueryProjectsTest.java index 98f9515..07fbf56 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticQueryProjectsTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticQueryProjectsTest.java
@@ -23,6 +23,7 @@ import com.google.inject.Injector; import java.util.concurrent.ExecutionException; import org.eclipse.jgit.lib.Config; +import org.junit.After; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -52,11 +53,22 @@ } } + private String testName() { + return testName.getMethodName().toLowerCase() + "_"; + } + + @After + public void cleanupIndex() { + if (nodeInfo != null) { + ElasticTestUtils.deleteAllIndexes(nodeInfo, testName()); + } + } + @Override protected Injector createInjector() { Config elasticsearchConfig = new Config(config); InMemoryModule.setDefaults(elasticsearchConfig); - String indicesPrefix = testName.getMethodName().toLowerCase() + "_"; + String indicesPrefix = testName(); ElasticTestUtils.configure(elasticsearchConfig, nodeInfo.port, indicesPrefix); ElasticTestUtils.createAllIndexes(nodeInfo, indicesPrefix); return Guice.createInjector(new InMemoryModule(elasticsearchConfig, notesMigration));
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java index 21609b5..ed21e6e 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
@@ -110,8 +110,46 @@ return new ElasticNodeInfo(node, elasticDir, getHttpPort(node)); } - static void deleteAllIndexes(ElasticNodeInfo nodeInfo) { - nodeInfo.node.client().admin().indices().prepareDelete("_all").execute().actionGet(); + static void deleteAllIndexes(ElasticNodeInfo nodeInfo, String prefix) { + Schema<ChangeData> changeSchema = ChangeSchemaDefinitions.INSTANCE.getLatest(); + nodeInfo + .node + .client() + .admin() + .indices() + .prepareDelete(String.format("%s%s_%04d", prefix, CHANGES, changeSchema.getVersion())) + .execute() + .actionGet(); + + Schema<AccountState> accountSchema = AccountSchemaDefinitions.INSTANCE.getLatest(); + nodeInfo + .node + .client() + .admin() + .indices() + .prepareDelete(String.format("%s%s_%04d", prefix, ACCOUNTS, accountSchema.getVersion())) + .execute() + .actionGet(); + + Schema<InternalGroup> groupSchema = GroupSchemaDefinitions.INSTANCE.getLatest(); + nodeInfo + .node + .client() + .admin() + .indices() + .prepareDelete(String.format("%s%s_%04d", prefix, GROUPS, groupSchema.getVersion())) + .execute() + .actionGet(); + + Schema<ProjectData> projectSchema = ProjectSchemaDefinitions.INSTANCE.getLatest(); + nodeInfo + .node + .client() + .admin() + .indices() + .prepareDelete(String.format("%s%s_%04d", prefix, PROJECTS, projectSchema.getVersion())) + .execute() + .actionGet(); } static class NodeInfo {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html index 09dcd84..95d6b46 100644 --- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html
@@ -46,7 +46,6 @@ const sectionSelectors = [ 'section.assignee', - 'section.labelStatus', 'section.strategy', 'section.topic', ]; @@ -85,7 +84,6 @@ deleteVote() { return Promise.resolve({ok: true}); }, }); stub('gr-change-metadata', { - _computeShowLabelStatus() { return true; }, _computeShowReviewersByState() { return true; }, }); });
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html index dd1f6be..e5728d8 100644 --- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html +++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -32,6 +32,7 @@ <link rel="import" href="../../shared/gr-linked-chip/gr-linked-chip.html"> <link rel="import" href="../../shared/gr-tooltip-content/gr-tooltip-content.html"> <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html"> +<link rel="import" href="../gr-change-requirements/gr-change-requirements.html"> <link rel="import" href="../gr-commit-info/gr-commit-info.html"> <link rel="import" href="../gr-reviewer-list/gr-reviewer-list.html"> @@ -94,26 +95,13 @@ .negative { background-color: var(--vote-color-negative); } - .labelStatus .value { - max-width: 9em; - } - .labelStatus li { - list-style-type: disc; - } .webLink { display: block; } - #missingLabels { - padding-left: 1.5em; - } - /* CSS Mixins should be applied last. */ section.assignee { @apply --change-metadata-assignee; } - section.labelStatus { - @apply --change-metadata-label-status; - } section.strategy { @apply --change-metadata-strategy; } @@ -349,26 +337,11 @@ </span> </section> </template> - <template is="dom-if" if="[[_showLabelStatus]]"> - <section class="labelStatus"> - <span class="title">Label Status</span> + <template is="dom-if" if="[[_showRequirements]]"> + <section class="requirementsStatus"> + <span class="title">Submit Status</span> <span class="value"> - <div hidden$="[[!_isWip]]"> - Work in progress - </div> - <div hidden$="[[!_showMissingLabels(missingLabels)]]"> - [[_computeMissingLabelsHeader(missingLabels)]] - <ul id="missingLabels"> - <template - is="dom-repeat" - items="[[missingLabels]]"> - <li>[[item]]</li> - </template> - </ul> - </div> - <div hidden$="[[_showMissingRequirements(missingLabels, _isWip)]]"> - Ready to submit - </div> + <gr-change-requirements change="[[change]]"></gr-change-requirements> </span> </section> </template>
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js index 0d23a16..aaada4f 100644 --- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js +++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -49,7 +49,6 @@ /** @type {?} */ revision: Object, commitInfo: Object, - missingLabels: Array, mutable: Boolean, /** * @type {{ note_db_enabled: string }} @@ -73,9 +72,9 @@ type: Boolean, computed: '_computeShowReviewersByState(serverConfig)', }, - _showLabelStatus: { + _showRequirements: { type: Boolean, - computed: '_computeShowLabelStatus(change)', + computed: '_computeShowRequirements(change)', }, _assignee: Array, @@ -286,6 +285,19 @@ return !!serverConfig.note_db_enabled; }, + _computeShowRequirements(change) { + if (change.status !== this.ChangeStatus.NEW) { + // TODO(maximeg) change this to display the stored + // requirements, once it is implemented server-side. + return false; + } + const hasRequirements = !!change.requirements && + Object.keys(change.requirements).length > 0; + const hasLabels = !!change.labels && + Object.keys(change.labels).length > 0; + return hasRequirements || hasLabels || !!change.work_in_progress; + }, + /** * A user is able to delete a vote iff the mutable property is true and the * reviewer that left the vote exists in the list of removable_reviewers @@ -355,25 +367,6 @@ }); }, - _computeShowLabelStatus(change) { - const isNewChange = change.status === this.ChangeStatus.NEW; - const hasLabels = Object.keys(change.labels).length > 0; - return isNewChange && hasLabels; - }, - - _computeMissingLabelsHeader(missingLabels) { - return 'Needs label' + - (missingLabels.length > 1 ? 's' : '') + ':'; - }, - - _showMissingLabels(missingLabels) { - return !!missingLabels.length; - }, - - _showMissingRequirements(missingLabels, workInProgress) { - return workInProgress || this._showMissingLabels(missingLabels); - }, - _computeProjectURL(project) { return Gerrit.Nav.getUrlForProjectChanges(project); },
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html index 6a04fd7..1330451 100644 --- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
@@ -65,6 +65,43 @@ 'Rebase Always'); }); + test('computed fields requirements', () => { + assert.isFalse(element._computeShowRequirements({status: 'MERGED'})); + assert.isFalse(element._computeShowRequirements({status: 'ABANDONED'})); + + // No labels and no requirements: submit status is useless + assert.isFalse(element._computeShowRequirements({ + status: 'NEW', + labels: {}, + })); + + // Work in Progress: submit status should be present + assert.isTrue(element._computeShowRequirements({ + status: 'NEW', + labels: {}, + work_in_progress: true, + })); + + // We have at least one reason to display Submit Status + assert.isTrue(element._computeShowRequirements({ + status: 'NEW', + labels: { + Verified: { + approved: false, + }, + }, + requirements: [], + })); + assert.isTrue(element._computeShowRequirements({ + status: 'NEW', + labels: {}, + requirements: [{ + fallback_text: 'Resolve all comments', + status: 'OK', + }], + })); + }); + test('show strategy for open change', () => { element.change = {status: 'NEW', submit_type: 'CHERRY_PICK', labels: {}}; flushAsynchronousOperations(); @@ -92,26 +129,6 @@ assert.isTrue(hasCc()); }); - test('computes submit status', () => { - let showMissingLabels = false; - sandbox.stub(element, '_showMissingLabels', () => { - return showMissingLabels; - }); - assert.isFalse(element._showMissingRequirements(null, false)); - assert.isTrue(element._showMissingRequirements(null, true)); - showMissingLabels = true; - assert.isTrue(element._showMissingRequirements(null, false)); - }); - - test('show missing labels', () => { - let missingLabels = []; - assert.isFalse(element._showMissingLabels(missingLabels)); - missingLabels = ['test']; - assert.isTrue(element._showMissingLabels(missingLabels)); - missingLabels.push('test2'); - assert.isTrue(element._showMissingLabels(missingLabels)); - }); - test('weblinks use Gerrit.Nav interface', () => { const weblinksStub = sandbox.stub(Gerrit.Nav, '_generateWeblinks') .returns([{name: 'stubb', url: '#s'}]); @@ -168,20 +185,6 @@ assert.equal(element._computeWebLinks(element.commitInfo).length, 1); }); - test('determines whether to show "Ready to Submit" label', () => { - const showMissingSpy = sandbox.spy(element, '_showMissingRequirements'); - element.missingLabels = ['bojack']; - element.change = {status: 'NEW', submit_type: 'CHERRY_PICK', labels: { - test: { - all: [{_account_id: 1, name: 'bojack', value: 1}], - default_value: 0, - values: [], - }, - }}; - flushAsynchronousOperations(); - assert.isTrue(showMissingSpy.called); - }); - test('_computeShowUploader test for uploader', () => { const change = { change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html new file mode 100644 index 0000000..536b943 --- /dev/null +++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html
@@ -0,0 +1,75 @@ +<!-- +@license +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. +--> + +<link rel="import" href="../../../bower_components/polymer/polymer.html"> +<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html"> +<link rel="import" href="../../../styles/shared-styles.html"> + +<dom-module id="gr-change-requirements"> + <template strip-whitespace> + <style include="shared-styles"> + .status { + display: inline-block; + font-weight: initial; + text-align: center; + width: 1em; + } + .unsatisfied .status { + color: #FFA62F; + } + .satisfied .status { + color: #388E3C; + } + .requirement { + padding: .1em .3em; + } + .requirementContainer:not(:first-of-type) { + margin-top: .25em; + } + .labelName, .changeIsWip { + font-weight: bold; + } + </style> + <template is="dom-if" if="[[_showWip]]"> + <div class="requirement unsatisfied changeIsWip"> + <span class="status">⧗</span> + Work in Progress + </div> + </template> + <template is="dom-if" if="[[_showLabels]]"> + <template + is="dom-repeat" + items="[[labels]]"> + <div class$="requirement [[item.style]]"> + <span class="status">[[item.status]]</span> + Label <span class="labelName">[[item.label]]</span> + </div> + </template> + </template> + <template + is="dom-repeat" + items="[[requirements]]"> + <div class$="requirement [[_computeRequirementClass(item.satisfied)]]"> + <span class="status"> + [[_computeRequirementStatus(item.satisfied)]] + </span> + [[item.fallback_text]] + </div> + </template> + </template> + <script src="gr-change-requirements.js"></script> +</dom-module>
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js new file mode 100644 index 0000000..884e9cd --- /dev/null +++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
@@ -0,0 +1,102 @@ +/** + * @license + * 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. + */ +(function() { + 'use strict'; + + Polymer({ + is: 'gr-change-requirements', + + properties: { + /** @type {?} */ + change: Object, + requirements: { + type: Array, + computed: '_computeRequirements(change)', + }, + labels: { + type: Array, + computed: '_computeLabels(change)', + }, + _showWip: { + type: Boolean, + computed: '_computeShowWip(change)', + }, + _showLabels: { + type: Boolean, + computed: '_computeShowLabelStatus(change)', + }, + }, + + behaviors: [ + Gerrit.RESTClientBehavior, + ], + + _computeShowLabelStatus(change) { + return change.status === this.ChangeStatus.NEW; + }, + + _computeShowWip(change) { + return change.work_in_progress; + }, + + _computeRequirements(change) { + const _requirements = []; + + if (change.requirements) { + for (const requirement of change.requirements) { + requirement.satisfied = requirement.status === 'OK'; + _requirements.push(requirement); + } + } + + return _requirements; + }, + + _computeLabels(change) { + const labels = change.labels; + const _labels = []; + + for (const label in labels) { + if (!labels.hasOwnProperty(label)) { continue; } + const obj = labels[label]; + if (obj.optional) { continue; } + + const status = this._computeRequirementStatus(obj.approved); + const style = this._computeRequirementClass(obj.approved); + _labels.push({label, status, style}); + } + + return _labels; + }, + + _computeRequirementClass(requirementStatus) { + if (requirementStatus) { + return 'satisfied'; + } else { + return 'unsatisfied'; + } + }, + + _computeRequirementStatus(requirementStatus) { + if (requirementStatus) { + return '✓'; + } else { + return '⧗'; + } + }, + }); +})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html new file mode 100644 index 0000000..560a33d --- /dev/null +++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
@@ -0,0 +1,129 @@ +<!DOCTYPE html> +<!-- +@license +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. +--> + +<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes"> +<title>gr-change-requirements</title> + +<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script> +<script src="../../../bower_components/web-component-tester/browser.js"></script> +<link rel="import" href="../../../test/common-test-setup.html"/> +<link rel="import" href="gr-change-requirements.html"> + +<script>void(0);</script> + +<test-fixture id="basic"> + <template> + <gr-change-requirements></gr-change-requirements> + </template> +</test-fixture> + +<script> + suite('gr-change-metadata tests', () => { + let element; + + setup(() => { + element = fixture('basic'); + }); + + test('computed fields', () => { + assert.isTrue(element._computeShowLabelStatus({status: 'NEW'})); + assert.isFalse(element._computeShowLabelStatus({status: 'MERGED'})); + assert.isFalse(element._computeShowLabelStatus({status: 'ABANDONED'})); + + assert.isTrue(element._computeShowWip({work_in_progress: true})); + assert.isFalse(element._computeShowWip({work_in_progress: false})); + + assert.equal(element._computeRequirementClass(true), 'satisfied'); + assert.equal(element._computeRequirementClass(false), 'unsatisfied'); + + assert.equal(element._computeRequirementStatus(true), '✓'); + assert.equal(element._computeRequirementStatus(false), '⧗'); + }); + + test('properly converts satisfied labels', () => { + element.change = { + status: 'NEW', + labels: { + Verified: { + approved: true, + }, + }, + requirements: [], + }; + flushAsynchronousOperations(); + + const labelName = element.$$('.satisfied .labelName'); + assert.ok(labelName); + assert.isFalse(labelName.hasAttribute('hidden')); + assert.equal(labelName.innerHTML, 'Verified'); + }); + + test('properly converts unsatisfied labels', () => { + element.change = { + status: 'NEW', + labels: { + Verified: { + approved: false, + }, + }, + }; + flushAsynchronousOperations(); + + const labelName = element.$$('.unsatisfied .labelName'); + assert.ok(labelName); + assert.isFalse(labelName.hasAttribute('hidden')); + assert.equal(labelName.innerHTML, 'Verified'); + }); + + test('properly displays Work In Progress', () => { + element.change = { + status: 'NEW', + labels: {}, + requirements: [], + work_in_progress: true, + }; + flushAsynchronousOperations(); + + const changeIsWip = element.$$('.changeIsWip.unsatisfied'); + assert.ok(changeIsWip); + assert.isFalse(changeIsWip.hasAttribute('hidden')); + assert.notEqual(changeIsWip.innerHTML.indexOf('Work in Progress'), -1); + }); + + + test('properly displays a satisfied requirement', () => { + element.change = { + status: 'NEW', + labels: {}, + requirements: [{ + fallback_text: 'Resolve all comments', + status: 'OK', + }], + }; + flushAsynchronousOperations(); + + const satisfiedRequirement = element.$$('.satisfied'); + assert.ok(satisfiedRequirement); + assert.isFalse(satisfiedRequirement.hasAttribute('hidden')); + + // Extract the content of the text node (second element, after the span) + const textNode = satisfiedRequirement.childNodes[1].nodeValue.trim(); + assert.equal(textNode, 'Resolve all comments'); + }); + }); +</script>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html index 100c112..88cee4c 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -406,7 +406,6 @@ revision="[[_selectedRevision]]" commit-info="[[_commitInfo]]" server-config="[[_serverConfig]]" - missing-labels="[[_missingLabels]]" mutable="[[_loggedIn]]" parent-is-current="[[_parentIsCurrent]]" on-show-reply-dialog="_handleShowReplyDialog">
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js index 60eddf0b..7b31ee3 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -174,11 +174,6 @@ }, _loading: Boolean, /** @type {?} */ - _missingLabels: { - type: Array, - computed: '_computeMissingLabels(_change.labels)', - }, - /** @type {?} */ _projectConfig: Object, _rebaseOnCurrent: Boolean, _replyButtonLabel: { @@ -384,18 +379,6 @@ this._editingCommitMessage = false; }, - _computeMissingLabels(labels) { - const missingLabels = []; - for (const label in labels) { - if (!labels.hasOwnProperty(label)) { continue; } - const obj = labels[label]; - if (!obj.optional && !obj.approved) { - missingLabels.push(label); - } - } - return missingLabels; - }, - _computeChangeStatusChips(change, mergeable) { // Show no chips until mergeability is loaded. if (mergeable === null || mergeable === undefined) { return []; }
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html index 898f9ea..a2a2de7 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -464,22 +464,6 @@ assert.equal(statusChips.length, 2); }); - test('_computeMissingLabels', () => { - let labels = {}; - assert.equal(element._computeMissingLabels(labels).length, 0); - labels = {test: {}}; - assert.deepEqual(element._computeMissingLabels(labels), ['test']); - labels.test.approved = true; - assert.equal(element._computeMissingLabels(labels).length, 0); - labels.test.approved = false; - labels.test.optional = true; - assert.equal(element._computeMissingLabels(labels).length, 0); - labels.test.optional = false; - labels.test2 = {}; - assert.deepEqual(element._computeMissingLabels(labels), - ['test', 'test2']); - }); - test('diff preferences open when open-diff-prefs is fired', () => { const overlayOpenStub = sandbox.stub(element.$.fileList, 'openDiffPrefs'); @@ -1611,8 +1595,6 @@ fireEdit = () => { element.$.actions.dispatchEvent(new CustomEvent('edit-tap')); }; - sandbox.stub(element.$.metadata, '_computeShowLabelStatus'); - sandbox.stub(element.$.metadata, '_computeLabelNames'); navigateToChangeStub.restore(); element._change = {revisions: {rev1: {_number: 1}}}; @@ -1664,7 +1646,6 @@ }); test('_handleStopEditTap', done => { - sandbox.stub(element.$.metadata, '_computeShowLabelStatus'); sandbox.stub(element.$.metadata, '_computeLabelNames'); navigateToChangeStub.restore(); sandbox.stub(element, 'computeLatestPatchNum').returns(1);