Page MenuHomePhabricator

ArcanistUnitTestResult serializes the userdata field as "userData", while the diff workflow serializes it as "userdata"
Open, Needs TriagePublic

Description

Basically ArcanistUnitTestResult looks like:

public function toDictionary() {
  return array(
    'name' => $this->getName(),
    'link' => $this->getLink(),
    'result' => $this->getResult(),
    'duration' => $this->getDuration(),
    'extra' => $this->getExtraData(),
    'userData' => $this->getUserData(),
    'coverage' => $this->getCoverage(),
  );
}

while the ArcanistDiffWorkflow does this:

$this->testResults = array();
foreach ($unit_workflow->getTestResults() as $test) {
  $this->testResults[] = array(
    'name'      => $test->getName(),
    'link'      => $test->getLink(),
    'result'    => $test->getResult(),
    'userdata'  => $test->getUserData(),
    'coverage'  => $test->getCoverage(),
    'extra'     => $test->getExtraData(),
  );
}

Taking the output from arc unit --output json is almost perfect, except the capitalization of "userData" means that this field doesn't match (even though all of the other fields are fine).

Event Timeline

hach-que assigned this task to epriestley.
hach-que raised the priority of this task from to Needs Triage.
hach-que updated the task description. (Show Details)
hach-que added a project: Arcanist.
hach-que added a subscriber: hach-que.

Can we just replace the second one with $test->toDictionary()? I'm pretty sure this is just written dumb and I'm happy to deal with anything that breaks because of the "d".

We can. This means that duration and extra will also get stored in Phabricator (which would be useful for displaying the duration of each test in the UI in the future)

Oh, I thought they were the other way around. I'm surprised "duration" is being output via --json but not send to Phabricator.

We should probably switch to "userdata", then. Submitting a new "duration" field seems fine to me (I think that may have been the intent).

Yeah, the JSON blobs in Phabricator use "userdata" so I'd expect Arcanist to change to that.