Page MenuHomePhabricator

operations/puppet patches having CR+2 are not retested on a rebase
Closed, ResolvedPublic

Description

When an event occur on a change having Code-review +2, it is enqueued in gate-and-submit and would eventually be submitted in Gerrit.

Since T105474 / 0f0aecfb695e668786ac9847dfed63f79d8fcca6 , a change having CR+2 would not be rechecked at all.

On the operations/puppet that means such a change would not have any test run again for example when doing a rebase. An example is https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/509888/

Patchset 2 got a Code-Review +2, the rebase had the vote sticking which filters it out from the test-prio pipeline.

The workaround is to remove the Code-Review +2 label and recheck.

Details

Related Gerrit Patches:
integration/config : masterRevert "zuul: skip test/test-prio for CR+2 changes"
operations/puppet : refs/meta/configAllow tests to run on trivial rebase

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 14 2019, 9:39 AM

Change 510112 had a related patch set uploaded (by Hashar; owner: Hashar):
[operations/puppet@refs/meta/config] Allow tests to run on trivial rebase

https://gerrit.wikimedia.org/r/510112

jbond added a subscriber: jbond.May 14 2019, 11:00 AM

Relevant Gerrit code:

gerrit-server/src/main/java/com/google/gerrit/server/ApprovalCopier.java
private static boolean canCopy(
    ProjectState project, PatchSetApproval psa, PatchSet.Id psId, ChangeKind kind) {
  int n = psa.getKey().getParentKey().get();
  checkArgument(n != psId.get());
  LabelType type = project.getLabelTypes().byLabel(psa.getLabelId());
  if (type == null) {
    return false;
  } else if ((type.isCopyMinScore() && type.isMaxNegative(psa))
      || (type.isCopyMaxScore() && type.isMaxPositive(psa))) {
    return true;
  }
  switch (kind) {
    case MERGE_FIRST_PARENT_UPDATE:
      return type.isCopyAllScoresOnMergeFirstParentUpdate();
    case NO_CODE_CHANGE:
      return type.isCopyAllScoresIfNoCodeChange();
    case TRIVIAL_REBASE:
      return type.isCopyAllScoresOnTrivialRebase();
    case NO_CHANGE:
      return type.isCopyAllScoresIfNoChange()
          || type.isCopyAllScoresOnTrivialRebase()
          || type.isCopyAllScoresOnMergeFirstParentUpdate()
          || type.isCopyAllScoresIfNoCodeChange();
    case REWORK:
    default:
      return false;
  }
}

The labels setting copyMaxScore (doc), defaults to false.

When setting it to true that ensure it is always copied. Setting it to false does not shortcircuit any logic and the other CopyAll* are applied thus the CR+2 get copied :-\

Change 510112 abandoned by Hashar:
Allow tests to run on trivial rebase

https://gerrit.wikimedia.org/r/510112

hashar added a comment.EditedMay 14 2019, 12:25 PM
zuul.Scheduler: Processing trigger event <TriggerEvent patchset-created operations/puppet production 509888,3>

The rebase causes a patchset-created event which is matched:

zuul.IndependentPipelineManager:
Event <TriggerEvent patchset-created operations/puppet production 509888,3>
for change <Change 0x7fadd686ea10 509888,3>
matched <EventFilter types: patchset-created branches: (?!^refs/meta/config) ignore_deletes: True emails: <bunch of emails>>
^^^^^
in pipeline <IndependentPipelineManager test-prio>

The change is added to the pipeline test-prio

zuul.Scheduler: Adding operations/puppet, <Change 0x7fadd686ea10 509888,3> to <Pipeline test-prio>

But it rejects the change since it has a CodeReview +2 event:

zuul.IndependentPipelineManager:
Considering adding change <Change 0x7fadd686ea10 509888,3>

Change <Change 0x7fadd686ea10 509888,3>
does not match pipeline requirement <ChangeishFilter reject_approvals: [{'code-review': 2}]>
^^^^^^^^^^^^^^

Same issue as T223212

Change 510148 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] Revert "zuul: skip test/test-prio for CR+2 changes"

https://gerrit.wikimedia.org/r/510148

Change 510148 merged by jenkins-bot:
[integration/config@master] Revert "zuul: skip test/test-prio for CR+2 changes"

https://gerrit.wikimedia.org/r/510148

hashar closed this task as Resolved.May 14 2019, 12:34 PM
hashar claimed this task.

I have reverted the change that caused the issue. Sorry for the inconvenience.