Paths

Table of Contentst

Differential D14495

Prevent double rendering of unit test results
ClosedPublic
Actions

Authored by joshuaspence on Nov 16 2015, 7:48 PM.

Tags

None

Referenced Files

	F13052126: D14495.diff
	Fri, Apr 19, 7:15 AM

	F13049687: D14495.diff
	Fri, Apr 19, 2:23 AM

	Unknown Object (File)
	Wed, Apr 17, 10:35 PM

	Unknown Object (File)
	Sat, Apr 6, 12:01 AM

	Unknown Object (File)
	Wed, Apr 3, 6:48 AM

	Unknown Object (File)
	Wed, Apr 3, 5:52 AM

	Unknown Object (File)
	Sat, Mar 30, 12:51 AM

	Unknown Object (File)
	Thu, Mar 21, 12:58 PM

Subscribers

glotechsol-troy

Details

Reviewers

Group Reviewers

Blessed Reviewers

Commits

rARCa4dba24c468c: Prevent double rendering of unit test results

Summary

D14037 had the logic slightly wrong and unit test results are now being double rendered.

Test Plan

Ran a unit test with arc unit -- path/to/test and saw the results rendered only once.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

joshuaspence updated this revision to Diff 35068.Nov 16 2015, 7:48 PM

joshuaspence retitled this revision from to Prevent double rendering of unit test results.

joshuaspence updated this object.

joshuaspence edited the test plan for this revision. (Show Details)

joshuaspence added a reviewer: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptNov 16 2015, 7:48 PM

Herald added a subscriber: Korvin. · View Herald Transcript

Maybe add a comment explaining why !$thing causes $thing to occur.

This revision is now accepted and ready to land.Nov 16 2015, 8:52 PM

Closed by commit rARCa4dba24c468c: Prevent double rendering of unit test results (authored by joshuaspence, committed by joshuaspence). · Explain WhyNov 17 2015, 8:20 PM

This revision was automatically updated to reflect the committed changes.

@epriestley's comment shows why shouldEchoTestResults is a bad name for this method. A name like testEngineWillEchoTestResults is also awful, but it better communicates the purpose. The problem is that there is no subject in the name. It would be clearer if the method were rendererShouldEchoTestResults and the logic were inverted.

Firehed mentioned this in D15880: Move 'should render' logic to PhutilUnitTestEngine.May 10 2016, 7:09 PM

urzds mentioned this in T11479: ArcanistUnitTestEngine->shouldEchoTestResults() logic inverted.Aug 15 2016, 9:02 PM

Revision Contents
Changeset List

Path

Size

src/

unit/

engine/

ArcanistConfigurationDrivenUnitTestEngine.php

4 lines

Diff 35084

src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php

Loading...