Improve Jest phabricator plugin (jestjs#4195)

tushardhole · Aug 4, 2017 · 50cf6f9 · 50cf6f9
1 parent da8c8ca
commit 50cf6f9
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 36 deletions.
diff --git a/packages/jest-phabricator/example/JestUnitTestEngine.php b/packages/jest-phabricator/example/JestUnitTestEngine.php
@@ -28,7 +28,7 @@ private function getFutureResults($future) {
       $raw_results = json_decode(
         Filesystem::readFile($output_JSON),
         true
-      )['phabricatorReport'];
+      )['coverageMap'];
       Filesystem::remove($output_JSON);
     } else {
       $raw_results = json_decode($stdout, true);

diff --git a/packages/jest-phabricator/src/index.js b/packages/jest-phabricator/src/index.js
@@ -9,16 +9,14 @@
 
 import type {
   AggregatedResult,
+  AggregatedResultWithoutCoverage,
   CoverageMap,
-  FormattedTestResult,
 } from 'types/TestResult';
 
-type PhabricatorReport = AggregatedResult & {
-  phabricatorReport: Array<FormattedTestResult>,
+type PhabricatorReport = AggregatedResultWithoutCoverage & {
+  coverageMap?: ?{[string]: string},
 };
 
-import {formatTestResults} from 'jest-util';
-
 function summarize(coverageMap: CoverageMap) {
   const summaries = Object.create(null);
 
@@ -29,8 +27,7 @@ function summarize(coverageMap: CoverageMap) {
     Object.keys(lineCoverage).forEach(lineNumber => {
       // Line numbers start at one
       const number = parseInt(lineNumber, 10) - 1;
-      const visited = !!lineCoverage[lineNumber];
-      covered[number] = visited ? 'C' : 'U';
+      covered[number] = lineCoverage[lineNumber] ? 'C' : 'U';
     });
 
     for (let i = 0; i < covered.length; i++) {
@@ -46,30 +43,8 @@ function summarize(coverageMap: CoverageMap) {
 }
 
 module.exports = function(results: AggregatedResult): PhabricatorReport {
-  const coverageMap = results.coverageMap && summarize(results.coverageMap);
-
-  const formatter = (coverage, reporter) => coverageMap;
-  const report = formatTestResults(results, formatter);
-
-  return {
-    aggregatedResult: results,
-    // Remove the coverageMap here as it uses a lot of memory.
-    coverageMap: null,
-    formattedTestResults: report.testResults,
-    numFailedTestSuites: results.numFailedTestSuites,
-    numFailedTests: results.numFailedTests,
-    numPassedTestSuites: results.numPassedTestSuites,
-    numPassedTests: results.numPassedTests,
-    numPendingTestSuites: results.numPendingTestSuites,
-    numPendingTests: results.numPendingTests,
-    numRuntimeErrorTestSuites: results.numRuntimeErrorTestSuites,
-    numTotalTestSuites: results.numTotalTestSuites,
-    numTotalTests: results.numTotalTests,
-    phabricatorReport: report.testResults,
-    snapshot: results.snapshot,
-    startTime: results.startTime,
-    success: results.success,
-    testResults: results.testResults,
-    wasInterrupted: results.wasInterrupted,
-  };
+  // $FlowFixMe: This should work, but it does not.
+  return Object.assign({}, results, {
+    coverageMap: results.coverageMap && summarize(results.coverageMap),
+  });
 };
diff --git a/types/TestResult.js b/types/TestResult.js
@@ -99,8 +99,7 @@ export type FormattedAssertionResult = {
   failureMessages: Array<string> | null,
 };
 
-export type AggregatedResult = {
-  coverageMap?: ?CoverageMap,
+export type AggregatedResultWithoutCoverage = {
   numFailedTests: number,
   numFailedTestSuites: number,
   numPassedTests: number,
@@ -117,6 +116,10 @@ export type AggregatedResult = {
   wasInterrupted: boolean,
 };
 
+export type AggregatedResult = AggregatedResultWithoutCoverage & {
+  coverageMap?: ?CoverageMap,
+};
+
 export type Suite = {|
   title: string,
   suites: Array<Suite>,