Merge branch 'stable-3.1'

* stable-3.1:
  Update git submodules
  Update git submodules
  gr-rest-api-interface.js: Remove unnecesary Promise resolution
  LuceneQueryChangesTest: Remove unneeded override of visible()
  e2e-tests: Rename the now reused json filename constant
  e2e-tests: Refactor documentation about functional
  RevisionActions: Do not alter server response
  e2e-tests: Move core json files into scala package
  e2e-tests: Support adding/running non-core scenarios
  e2e-tests: Add create/delete project to CloneUsingBothProtocols
  ChangeQueryBuilder: Throw error on ambiguous visibleto by display name

Change-Id: Idbb499461ecad11da49b3bcbe5d5bf14e90bfe31

Documentation/dev-e2e-tests.txt

@@ -1,29 +1,38 @@
 :linkattrs:
-= Gerrit Code Review - End to end load tests
+= Gerrit Code Review - End to end tests
 
-This document provides a description of a Gerrit load test scenario implemented using the
+This document provides descriptions of Gerrit end-to-end (`e2e`) test scenarios implemented using
 link:https://gatling.io/[Gatling,role=external,window=_blank] framework.
 
 Similar scenarios have been successfully used to compare performance of different Gerrit versions
-or study the Gerrit response under different load profiles.
+or study the Gerrit response under different load profiles. Although mostly for load, scenarios can
+either be for link:https://gatling.io/load-testing-continuous-integration/[load or functional,role=external,window=_blank]
+(e2e) testing purposes. Functional scenarios may then reuse this framework and Gatling's usability
+features such as its protocols (more below) and
+link:https://en.wikipedia.org/wiki/Domain-specific_language[DSL,role=external,window=_blank].
+
+That cross test-scope reusability applies to both Gerrit core scenarios and non-core ones, such as
+for Gerrit plugins or other potential extensions. End-to-end testing may then include scopes like
+feature integration, deployment, smoke (and load) testing. These load and functional test scopes
+should remain orthogonal to the unit and component (aka Gerrit `IT`-suffixed or `acceptance`) ones.
+The term `acceptance` though may still be coined by organizations to target e2e functional testing.
 
 == What is Gatling?
 
-Gatling is a load testing tool which provides out of the box support for the HTTP protocol.
+Gatling is mostly a load testing tool which provides out of the box support for the HTTP protocol.
 Documentation on how to write an HTTP load test can be found
 link:https://gatling.io/docs/current/http/http_protocol/[here,role=external,window=_blank].
-
-However, in the scenario we are proposing, we are leveraging the
-link:https://github.com/GerritForge/gatling-git[Gatling Git extension,role=external,window=_blank]
-to run tests at Git protocol level.
+However, in the scenarios that were initially proposed, the
+link:https://github.com/GerritForge/gatling-git[Gatling Git extension,role=external,window=_blank] was
+leveraged to run tests at the Git protocol level.
 
 Gatling is written in Scala, but the abstraction provided by the Gatling DSL makes the scenarios
 implementation easy even without any Scala knowledge. The
 link:https://gitenterprise.me/2019/12/20/stress-your-gerrit-with-gatling/[Stress your Gerrit with Gatling,role=external,window=_blank]
 blog post has more introductory information.
 
-Examples of scenarios can be found in the `e2e-tests` directory. The files in that directory
-should be formatted using the mainstream
+Examples of scenarios can be found in the `e2e-tests` directory. The files in that directory should
+be formatted using the mainstream
 link:https://plugins.jetbrains.com/plugin/1347-scala[Scala plugin for IntelliJ,role=external,window=_blank].
 The latter is not mandatory but preferred for `sbt` and Scala IDE purposes in this project.
 
@@ -86,10 +95,11 @@ ssh-keyscan -t rsa -p 29418 localhost > ~/.ssh/known_hosts
 === Input file
 
 The `CloneUsingBothProtocols` scenario is fed with the data coming from the
-`src/test/resources/data/CloneUsingBothProtocols.json` file. Such a file contains the commands and
-repository used during the load test. That file currently looks like below. This scenario serves
-as a simple example with no actual load in it. It can be used to test or validate the local setup.
-More complex scenarios can be further developed, under the `com.google.gerrit.scenarios` package.
+`src/test/resources/data/com/google/gerrit/scenarios/CloneUsingBothProtocols.json` file. Such a
+file contains the commands and repository used during the e2e test. That file currently looks like
+below. This scenario serves as a simple example with no actual load in it. It can be used to test
+or validate the local setup. More complex scenarios can be further developed, under the
+`com.google.gerrit.scenarios` package.
 
 ----
 [
@@ -111,9 +121,19 @@ Valid commands are:
 * `pull`
 * `push`
 
+=== Project and HTTP credentials
+
 The example above assumes that the `loadtest-repo` project exists in the Gerrit under test. The
-`HTTP Credentials` or password obtained from test user's `Settings` (in Gerrit) may be required, in
-`src/test/resources/application.conf`, depending on the above commands used.
+`CloneUsingBothProtocols` scenario already includes creating that project and deleting it once done
+with it. That scenario class can be used as an example of how a scenario can compose itself
+alongside other scenarios (here, `CreateProject` and `DeleteProject`).
+
+The `HTTP Credentials` or password obtained from test user's `Settings` (in Gerrit) may be
+required, in `src/test/resources/application.conf`, depending on the above commands used. That
+file's `http` section shows which shell environment variables can be used to set those credentials.
+
+Executing the `CloneUsingBothProtocols` scenario, as is, does require setting the http credentials.
+That is because of the aforementioned create/delete project (http) scenarios composed within it.
 
 == How to run tests
 
@@ -142,6 +162,27 @@ Gatling's logging level.
 docker run -it e2e-tests -s com.google.gerrit.scenarios.CloneUsingBothProtocols
 ----
 
+=== How to run non-core scenarios
+
+Locally adding non-core scenarios, for example from Gerrit plugins, is as simple as copying such
+files in. Copying is necessary over linking, unless running using Docker (above) is not required.
+Docker does not support links for files it has to copy over through the Dockerfile (here, the
+scenario files). Here is how to proceed for adding such external (e.g., plugin) scenario files in:
+
+----
+pushd e2e-tests/src/test/scala
+cp -r (or, ln -s) scalaPackageStructure .
+popd
+
+pushd e2e-tests/src/test/resources/data
+cp -r (or, ln -s) jsonFilesPackageStructure .
+popd
+----
+
+The destination folders above readily git-ignore every non-core scenario file added under them. If
+running using Docker, `e2e-tests/Dockerfile` may require another `COPY` line for the hereby added
+scenarios. Aforementioned `sbt` or `docker` commands can then be used to run the added tests.
+
 GERRIT
 ------
 Part of link:index.html[Gerrit Code Review]

Documentation/dev-readme.txt

@@ -65,8 +65,8 @@ see <<dev-bazel#tests,Running Unit Tests with Bazel>>.
 [[e2e]]
 === End-to-end tests
 
-<<dev-e2e-tests#,This document>> describes how load test scenarios are
-implemented using link:https://gatling.io/[`Gatling`].
+<<dev-e2e-tests#,This document>> describes how `e2e` (load or functional) test
+scenarios are implemented using link:https://gatling.io/[`Gatling`,role=external,window=_blank].
 
 
 == Local server

e2e-tests/src/test/resources/data/.gitignore

@@ -0,0 +1,4 @@
+*
+!*/
+!/com/google/gerrit/scenarios/*
+!/.gitignore

e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateProject.json

@@ -0,0 +1,5 @@
+[
+  {
+    "url": "http://localhost:8080/a/projects/loadtest-repo"
+  }
+]

e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/DeleteProject.json

@@ -0,0 +1,5 @@
+[
+  {
+    "url": "http://localhost:8080/a/projects/loadtest-repo/delete-project~delete"
+  }
+]

e2e-tests/src/test/scala/.gitignore

@@ -0,0 +1,4 @@
+*
+!*/
+!/com/google/gerrit/scenarios/*
+!/.gitignore

e2e-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala

@@ -15,18 +15,32 @@
 package com.google.gerrit.scenarios
 
 import io.gatling.core.Predef._
+import io.gatling.core.feeder.FileBasedFeederBuilder
 import io.gatling.core.structure.ScenarioBuilder
 
 import scala.concurrent.duration._
 
 class CloneUsingBothProtocols extends GitSimulation {
+  private val data: FileBasedFeederBuilder[Any]#F = jsonFile(resource).queue
 
   private val test: ScenarioBuilder = scenario(name)
       .feed(data)
-      .exec(request)
+      .exec(gitRequest)
+
+  private val createProject = new CreateProject
+  private val deleteProject = new DeleteProject
 
   setUp(
+    createProject.test.inject(
+      atOnceUsers(1)
+    ),
     test.inject(
+      nothingFor(1 second),
       constantUsersPerSec(1) during (2 seconds)
-    )).protocols(protocol)
+    ),
+    deleteProject.test.inject(
+      nothingFor(3 second),
+      atOnceUsers(1)
+    ),
+  ).protocols(gitProtocol, httpProtocol)
 }

e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateProject.scala

@@ -0,0 +1,32 @@
+// Copyright (C) 2020 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.scenarios
+
+import io.gatling.core.Predef._
+import io.gatling.core.feeder.FileBasedFeederBuilder
+import io.gatling.core.structure.ScenarioBuilder
+
+class CreateProject extends GerritSimulation {
+  private val data: FileBasedFeederBuilder[Any]#F = jsonFile(resource).queue
+
+  val test: ScenarioBuilder = scenario(name)
+      .feed(data)
+      .exec(httpRequest)
+
+  setUp(
+    test.inject(
+      atOnceUsers(1)
+    )).protocols(httpProtocol)
+}

e2e-tests/src/test/scala/com/google/gerrit/scenarios/DeleteProject.scala

@@ -0,0 +1,32 @@
+// Copyright (C) 2020 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.scenarios
+
+import io.gatling.core.Predef._
+import io.gatling.core.feeder.FileBasedFeederBuilder
+import io.gatling.core.structure.ScenarioBuilder
+
+class DeleteProject extends GerritSimulation {
+  private val data: FileBasedFeederBuilder[Any]#F = jsonFile(resource).queue
+
+  val test: ScenarioBuilder = scenario(name)
+      .feed(data)
+      .exec(httpRequest)
+
+  setUp(
+    test.inject(
+      atOnceUsers(1)
+    )).protocols(httpProtocol)
+}

e2e-tests/src/test/scala/com/google/gerrit/scenarios/GerritSimulation.scala

@@ -0,0 +1,34 @@
+// Copyright (C) 2020 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.scenarios
+
+import com.github.barbasa.gatling.git.GatlingGitConfiguration
+import io.gatling.core.Predef._
+import io.gatling.http.Predef.http
+import io.gatling.http.protocol.HttpProtocolBuilder
+import io.gatling.http.request.builder.HttpRequestBuilder
+
+class GerritSimulation extends Simulation {
+  implicit val conf: GatlingGitConfiguration = GatlingGitConfiguration()
+
+  private val path: String = this.getClass.getPackage.getName.replaceAllLiterally(".", "/")
+  protected val name: String = this.getClass.getSimpleName
+  protected val resource: String = s"data/$path/$name.json"
+
+  protected val httpRequest: HttpRequestBuilder = http(name).post("${url}")
+  protected val httpProtocol: HttpProtocolBuilder = http.basicAuth(
+    conf.httpConfiguration.userName,
+    conf.httpConfiguration.password)
+}

e2e-tests/src/test/scala/com/google/gerrit/scenarios/GitSimulation.scala

@@ -16,23 +16,18 @@ package com.google.gerrit.scenarios
 
 import java.io.{File, IOException}
 
+import com.github.barbasa.gatling.git.GitRequestSession
 import com.github.barbasa.gatling.git.protocol.GitProtocol
 import com.github.barbasa.gatling.git.request.builder.GitRequestBuilder
-import com.github.barbasa.gatling.git.{GatlingGitConfiguration, GitRequestSession}
 import io.gatling.core.Predef._
-import io.gatling.core.feeder.FileBasedFeederBuilder
 import org.apache.commons.io.FileUtils
 import org.eclipse.jgit.hooks.CommitMsgHook
 
-class GitSimulation extends Simulation {
-
-  implicit val conf: GatlingGitConfiguration = GatlingGitConfiguration()
+class GitSimulation extends GerritSimulation {
   implicit val postMessageHook: Option[String] = Some(s"hooks/${CommitMsgHook.NAME}")
 
-  protected val name: String = this.getClass.getSimpleName
-  protected val data: FileBasedFeederBuilder[Any]#F = jsonFile(s"data/$name.json").circular
-  protected val request = new GitRequestBuilder(GitRequestSession("${cmd}", "${url}"))
-  protected val protocol: GitProtocol = GitProtocol()
+  protected val gitRequest = new GitRequestBuilder(GitRequestSession("${cmd}", "${url}"))
+  protected val gitProtocol: GitProtocol = GitProtocol()
 
   after {
     Thread.sleep(5000)

e2e-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala

@@ -15,16 +15,18 @@
 package com.google.gerrit.scenarios
 
 import io.gatling.core.Predef._
+import io.gatling.core.feeder.FileBasedFeederBuilder
 import io.gatling.core.structure.ScenarioBuilder
 
 import scala.concurrent.duration._
 
 class ReplayRecordsFromFeeder extends GitSimulation {
+  private val data: FileBasedFeederBuilder[Any]#F = jsonFile(resource).circular
 
   private val test: ScenarioBuilder = scenario(name)
       .repeat(10000) {
         feed(data)
-            .exec(request)
+            .exec(gitRequest)
       }
 
   setUp(
@@ -34,6 +36,6 @@ class ReplayRecordsFromFeeder extends GitSimulation {
       rampUsers(10) during (5 seconds),
       constantUsersPerSec(20) during (15 seconds),
       constantUsersPerSec(20) during (15 seconds) randomized
-    )).protocols(protocol)
+    )).protocols(gitProtocol)
       .maxDuration(60 seconds)
 }

java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.server.query.change;
 
-import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.gerrit.entities.Change.CHANGE_ID_PATTERN;
 import static com.google.gerrit.server.account.AccountResolver.isSelf;
 import static com.google.gerrit.server.query.change.ChangeData.asChanges;
@@ -24,6 +23,7 @@ import static java.util.stream.Collectors.toSet;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Enums;
 import com.google.common.base.Splitter;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.primitives.Ints;
@@ -985,18 +985,23 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
     if (isSelf(who)) {
       return isVisible();
     }
+    Set<Account.Id> accounts = null;
     try {
-      return Predicate.or(
-          parseAccount(who).stream()
-              .map(a -> visibleto(args.userFactory.create(a)))
-              .collect(toImmutableList()));
+      accounts = parseAccount(who);
     } catch (QueryParseException e) {
       if (e instanceof QueryRequiresAuthException) {
         throw e;
       }
-      // Otherwise continue: if it's not an account, maybe it's a group?
+    }
+    if (accounts != null) {
+      if (accounts.size() == 1) {
+        return visibleto(args.userFactory.create(Iterables.getOnlyElement(accounts)));
+      } else if (accounts.size() > 1) {
+        throw error(String.format("\"%s\" resolves to multiple accounts", who));
+      }
     }
 
+    // If its not an account, maybe its a group?
     Collection<GroupReference> suggestions = args.groupBackend.suggest(who, null);
     if (!suggestions.isEmpty()) {
       HashSet<AccountGroup.UUID> ids = new HashSet<>();

javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -1784,16 +1784,43 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     assertQuery(q + " visibleto:self", change2, change1);
 
     // Second user cannot see first user's private change
-    Account.Id user2 = createAccount("anotheruser");
+    Account.Id user2 = createAccount("user2");
     assertQuery(q + " visibleto:" + user2.get(), change1);
-    assertQuery(q + " visibleto:anotheruser", change1);
+    assertQuery(q + " visibleto:user2", change1);
 
     String g1 = createGroup("group1", "Administrators");
-    gApi.groups().id(g1).addMembers("anotheruser");
+    gApi.groups().id(g1).addMembers("user2");
     assertQuery(q + " visibleto:" + g1, change1);
 
     requestContext.setContext(newRequestContext(user2));
     assertQuery("is:visible", change1);
+
+    Account.Id user3 = createAccount("user3");
+
+    // Explicitly authenticate user2 and user3 so that display name gets set
+    AuthRequest authRequest = AuthRequest.forUser("user2");
+    authRequest.setDisplayName("Another User");
+    authRequest.setEmailAddress("user2@example.com");
+    accountManager.authenticate(authRequest);
+    authRequest = AuthRequest.forUser("user3");
+    authRequest.setDisplayName("Another User");
+    authRequest.setEmailAddress("user3@example.com");
+    accountManager.authenticate(authRequest);
+
+    // Switch to user3
+    requestContext.setContext(newRequestContext(user3));
+    Change change3 = insert(repo, newChange(repo), user3);
+    Change change4 = insert(repo, newChangePrivate(repo), user3);
+
+    // User3 can see both their changes and the first user's change
+    assertQuery(q + " visibleto:" + user3.get(), change4, change3, change1);
+
+    // User2 cannot see user3's private change
+    assertQuery(q + " visibleto:" + user2.get(), change3, change1);
+
+    // Query as user3 by display name matching user2 and user3; bad request
+    assertFailingQuery(
+        q + " visibleto:\"Another User\"", "\"Another User\" resolves to multiple accounts");
   }
 
   @Test

polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js

@@ -496,9 +496,7 @@ class GrChangeActions extends mixinBehaviors( [
   }
 
   _getRebaseAction(revisionActions) {
-    return this._getRevisionAction(revisionActions, 'rebase',
-        {rebaseOnCurrent: null}
-    );
+    return this._getRevisionAction(revisionActions, 'rebase', null);
   }
 
   _getRevisionAction(revisionActions, actionName, emptyActionValue) {
@@ -523,7 +521,7 @@ class GrChangeActions extends mixinBehaviors( [
         .then(revisionActions => {
           if (!revisionActions) { return; }
 
-          this.revisionActions = this._updateRebaseAction(revisionActions);
+          this.revisionActions = revisionActions;
           this._sendShowRevisionActions({
             change: this.change,
             revisionActions,
@@ -548,18 +546,6 @@ class GrChangeActions extends mixinBehaviors( [
     );
   }
 
-  _updateRebaseAction(revisionActions) {
-    if (revisionActions && revisionActions.rebase) {
-      revisionActions.rebase.rebaseOnCurrent =
-          !!revisionActions.rebase.enabled;
-      this._parentIsCurrent = !revisionActions.rebase.enabled;
-      revisionActions.rebase.enabled = true;
-    } else {
-      this._parentIsCurrent = true;
-    }
-    return revisionActions;
-  }
-
   _changeChanged() {
     this.reload();
   }
@@ -1121,8 +1107,9 @@ class GrChangeActions extends mixinBehaviors( [
   }
 
   _calculateDisabled(action, hasKnownChainState) {
-    if (action.__key === 'rebase' && hasKnownChainState === false) {
-      return true;
+    if (action.__key === 'rebase') {
+      // Rebase button is only disabled when change has no parent(s).
+      return hasKnownChainState === false;
     }
     return !action.enabled;
   }
@@ -1602,6 +1589,13 @@ class GrChangeActions extends mixinBehaviors( [
     });
   }
 
+  _computeRebaseOnCurrent(revisionRebaseAction) {
+    if (revisionRebaseAction) {
+      return !!revisionRebaseAction.enabled;
+    }
+    return null;
+  }
+
   /**
    * Occasionally, a change created by a change action is not yet knwon to the
    * API for a brief time. Wait for the given change number to be recognized.

polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_html.js

@@ -101,7 +101,7 @@ export const htmlTemplate = html`
         </gr-dropdown>
     </div>
     <gr-overlay id="overlay" with-backdrop="">
-      <gr-confirm-rebase-dialog id="confirmRebase" class="confirmDialog" change-number="[[change._number]]" on-confirm="_handleRebaseConfirm" on-cancel="_handleConfirmDialogCancel" branch="[[change.branch]]" has-parent="[[hasParent]]" rebase-on-current="[[_revisionRebaseAction.rebaseOnCurrent]]" hidden=""></gr-confirm-rebase-dialog>
+      <gr-confirm-rebase-dialog id="confirmRebase" class="confirmDialog" change-number="[[change._number]]" on-confirm="_handleRebaseConfirm" on-cancel="_handleConfirmDialogCancel" branch="[[change.branch]]" has-parent="[[hasParent]]" rebase-on-current="[[_computeRebaseOnCurrent(_revisionRebaseAction)]]" hidden=""></gr-confirm-rebase-dialog>
       <gr-confirm-cherrypick-dialog id="confirmCherrypick" class="confirmDialog" change-status="[[changeStatus]]" commit-message="[[commitMessage]]" commit-num="[[commitNum]]" on-confirm="_handleCherrypickConfirm" on-cancel="_handleConfirmDialogCancel" project="[[change.project]]" hidden=""></gr-confirm-cherrypick-dialog>
       <gr-confirm-cherrypick-conflict-dialog id="confirmCherrypickConflict" class="confirmDialog" on-confirm="_handleCherrypickConflictConfirm" on-cancel="_handleConfirmDialogCancel" hidden=""></gr-confirm-cherrypick-conflict-dialog>
       <gr-confirm-move-dialog id="confirmMove" class="confirmDialog" on-confirm="_handleMoveConfirm" on-cancel="_handleConfirmDialogCancel" project="[[change.project]]" hidden=""></gr-confirm-move-dialog>

polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html

@@ -393,7 +393,7 @@ suite('gr-change-actions tests', () => {
 
       action.enabled = false;
       assert.equal(
-          element._calculateDisabled(action, hasKnownChainState), true);
+          element._calculateDisabled(action, hasKnownChainState), false);
     });
 
     test('rebase change', done => {
@@ -416,7 +416,6 @@ suite('gr-change-actions tests', () => {
         };
         assert.isTrue(fetchChangesStub.called);
         element._handleRebaseConfirm({detail: {base: '1234'}});
-        rebaseAction.rebaseOnCurrent = true;
         assert.deepEqual(fireActionStub.lastCall.args,
             ['/rebase', rebaseAction, true, {base: '1234'}]);
         done();
@@ -1924,57 +1923,23 @@ suite('gr-change-actions tests', () => {
       assert.strictEqual(element.$.confirmRebase.rebaseOnCurrent, null);
     });
 
-    test('_updateRebaseAction sets _parentIsCurrent on no rebase', () => {
-      const currentRevisionActions = {
-        cherrypick: {
-          enabled: true,
-          label: 'Cherry Pick',
-          method: 'POST',
-          title: 'cherrypick',
-        },
+    test('_computeRebaseOnCurrent', () => {
+      const rebaseAction = {
+        enabled: true,
+        label: 'Rebase',
+        method: 'POST',
+        title: 'Rebase onto tip of branch or parent change',
       };
-      element._parentIsCurrent = undefined;
-      element._updateRebaseAction(currentRevisionActions);
-      assert.isTrue(element._parentIsCurrent);
-    });
 
-    test('_updateRebaseAction', () => {
-      const currentRevisionActions = {
-        cherrypick: {
-          enabled: true,
-          label: 'Cherry Pick',
-          method: 'POST',
-          title: 'cherrypick',
-        },
-        rebase: {
-          enabled: true,
-          label: 'Rebase',
-          method: 'POST',
-          title: 'Rebase onto tip of branch or parent change',
-        },
-      };
-      element._parentIsCurrent = undefined;
-
-      // Rebase enabled should always end up true.
       // When rebase is enabled initially, rebaseOnCurrent should be set to
       // true.
-      assert.equal(element._updateRebaseAction(currentRevisionActions),
-          currentRevisionActions);
+      assert.isTrue(element._computeRebaseOnCurrent(rebaseAction));
 
-      assert.isTrue(currentRevisionActions.rebase.enabled);
-      assert.isTrue(currentRevisionActions.rebase.rebaseOnCurrent);
-      assert.isFalse(element._parentIsCurrent);
-
-      delete currentRevisionActions.rebase.enabled;
+      delete rebaseAction.enabled;
 
       // When rebase is not enabled initially, rebaseOnCurrent should be set to
       // false.
-      assert.equal(element._updateRebaseAction(currentRevisionActions),
-          currentRevisionActions);
-
-      assert.isTrue(currentRevisionActions.rebase.enabled);
-      assert.isFalse(currentRevisionActions.rebase.rebaseOnCurrent);
-      assert.isTrue(element._parentIsCurrent);
+      assert.isFalse(element._computeRebaseOnCurrent(rebaseAction));
     });
   });
 });

polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js

@@ -292,7 +292,6 @@ class GrChangeView extends mixinBehaviors( [
       _loading: Boolean,
       /** @type {?} */
       _projectConfig: Object,
-      _rebaseOnCurrent: Boolean,
       _replyButtonLabel: {
         type: String,
         value: 'Reply',

polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js

@@ -1218,15 +1218,7 @@ class GrRestApiInterface extends mixinBehaviors( [
       patchNum,
       reportEndpointAsIs: true,
     };
-    return this._getChangeURLAndFetch(req).then(revisionActions => {
-      // The rebase button on change screen is always enabled.
-      if (revisionActions.rebase) {
-        revisionActions.rebase.rebaseOnCurrent =
-            !!revisionActions.rebase.enabled;
-        revisionActions.rebase.enabled = true;
-      }
-      return revisionActions;
-    });
+    return this._getChangeURLAndFetch(req);
   }
 
   /**

polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html

@@ -332,36 +332,6 @@ suite('gr-rest-api-interface tests', () => {
         ]);
   });
 
-  suite('rebase action', () => {
-    let resolve_fetchJSON;
-    setup(() => {
-      sandbox.stub(element._restApiHelper, 'fetchJSON').returns(
-          new Promise(resolve => {
-            resolve_fetchJSON = resolve;
-          }));
-    });
-
-    test('no rebase on current', done => {
-      element.getChangeRevisionActions('42', '1337').then(
-          response => {
-            assert.isTrue(response.rebase.enabled);
-            assert.isFalse(response.rebase.rebaseOnCurrent);
-            done();
-          });
-      resolve_fetchJSON({rebase: {}});
-    });
-
-    test('rebase on current', done => {
-      element.getChangeRevisionActions('42', '1337').then(
-          response => {
-            assert.isTrue(response.rebase.enabled);
-            assert.isTrue(response.rebase.rebaseOnCurrent);
-            done();
-          });
-      resolve_fetchJSON({rebase: {enabled: true}});
-    });
-  });
-
   test('server error', done => {
     const getResponseObjectStub = sandbox.stub(element, 'getResponseObject');
     window.fetch.returns(Promise.resolve({ok: false}));