Page MenuHomePhabricator

[Wilds] Tailor the behavior of some unit tests which `print_r($the_entire_world, true)`
ClosedPublic

Authored by epriestley on Sep 25 2018, 10:42 PM.

Details

Summary

Ref T13098. I've eventually made arc unit at least sort of run, but a few unit tests have an issue under the new code.

We have a couple of cases where we print_r($stack, true) or print_r($exception, true), which is effectively the same because exceptions include a stack. In one case, we search in the stack for values to make sure PhutilOpaqueEnvelope is really masking secrets. In another case, we just use print_r(...) to label test cases, but some test data is exceptions.

Under the new code, the arc unit stack has access to a much larger "world" via variables reachable in stack frames, since it's connected to all toolsets/workflows and those objects are more deeply interconnected. This makes the output from print_r($stack) enormous and slow (20+ seconds) because of all the recursive referencing of complex variables.

In the case where we're looking at the stack, just print the last couple frames. Also add some positive code that searches for a known value in the stack.

In the case where we're describing variables, just drop the code. We don't really need to label these test cases, the "expect" value is sufficiently clear on its own.

Test Plan

Later, ran arc unit and saw it finish in a reasonable amount of time instead of hanging forever on these tests.

Diff Detail

Repository
rARC Arcanist
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley requested review of this revision.Sep 25 2018, 10:42 PM
epriestley created this revision.
epriestley marked an inline comment as done.Sep 26 2018, 3:02 PM
epriestley added inline comments.
src/error/__tests__/PhutilOpaqueEnvelopeTestCase.php
41

(This is debugging stuff which I cleaned up locally after actually getting tests to run.)

epriestley updated this revision to Diff 47096.Sep 26 2018, 3:03 PM
  • Remove the debugging code in this change.
amckinley accepted this revision.Sep 26 2018, 6:08 PM
amckinley added inline comments.
src/error/__tests__/PhutilOpaqueEnvelopeTestCase.php
41

Why not just $this->assertTrue(strpos($trace, $signpost)?

This revision is now accepted and ready to land.Sep 26 2018, 6:08 PM

assertTrue(...) requires the argument be exactly true (the boolean value), not just any "truthy" value, and the result will be something like 323 (the "needle" string is located at character offset 323 of the "haystack" string).

The result may also be "0" to mean "the haystack string begins with the needle string".

This revision was automatically updated to reflect the committed changes.