Page MenuHomePhabricator

Configure test-plan per repository
Closed, WontfixPublic

Description

If multiple teams are using Phabricator it is common situation each team have own way & rules how to work. Some our teams are asking make "test plan" mandatory for their projects while keeping global value as false.

We should make this field configurable per repository. I see repositories do not have custom fields support now. How could be this implemented?

Event Timeline

Pawka updated the task description. (Show Details)
Pawka added a project: Differential.
Pawka updated the task description. (Show Details)
Pawka added a subscriber: Pawka.
chad renamed this task from Configure test-plan per repository. to Configure test-plan per repository.Sep 3 2015, 3:37 PM
chad added a project: Diffusion.

Broadly, the behavior of fields can not currently be dependent on the repository a revision belongs to.

In the general case, on the web workflow, we render the "Test Plan" field before a repository is set, and need to know if the field is required or not to show a "Required" tip next to it. I suspect there are other sequencing issues elsewhere.

You can copy the TestPlan field to CustomTestPlan, disable the old field, and then try to hack up the new field to work. It won't work 100% but you might be able to get close enough.

You can try something like this (totally untested) by dropping it in phabricator/src/extensions/:

WarningCustomField.php
<?php

final class WarningCustomField extends DifferentialCustomField {

  public function getFieldKey() {
    return 'mycompany:mycustomfield';
  }

  public function shouldAppearInPropertyView() {
    // This is required or the field won't be loaded on the detail page.
    return true;
  }

  public function renderPropertyViewValue(array $handles) {
    // This prevents it from actually rendering a property.
    return null;
  }

  public function getWarningsForRevisionHeader(array $handles) {
    $warnings = array();

    $revision = $this->getObject();

    // Check if the revision belongs to a repository where test plans are
    // required but does not have one.


    $test_plan = $revision->getTestPlan();
    if (strlen($test_plan)) {
      // We have a test plan.
      return $warnings;
    }

    $repository = $revision->getRepository();
    if ($repository) {
      switch ($repository->getCallsign()) {
        case 'A':
        case 'B':
          // Test plans are optional in these repositories.
          return $warnings;
      }
    }

    $warnings[] = pht(
      'This revision belongs to a repository where a test plan is required, '.
      'but it does not have one.');

    return $warnings;
  }

}

That will do this:

Screen Shot 2015-09-04 at 10.05.27 AM.png (989×1 px, 160 KB)

However, it won't stop users from creating the revision in the first place.

epriestley claimed this task.

I don't currently plan to try to let fields be configured per-repository. This would be a lot of work to accommodate a fairly rare need, require substantial workflow changes, and doesn't seem to be particularly valuable. Workarounds like the one above (or just writing similar Herald rules) seem reasonable to me, if not ideal.

Workarounds like the one above (or just writing similar Herald rules) seem reasonable to me, if not ideal.

What are "similar Herald rules"? Is this possible today?

Oh, I guess there isn't actually a "test plan" Herald field. We could add one easily enough. I was thinking:

When:
[ Test Plan ] [ matches regexp ] [ /^\z/ ]
Take these actions:
(Do Something)

Where "Do something" could maybe be "add blocking reviewers" with a project named "WRITE A TEST PLAN YOU GOOF".