Page MenuHomePhorge

Update PhutilCowsay.php to work for small cows
ClosedPublic

Authored by Sten on Sep 12 2023, 16:09.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 26, 05:14
Unknown Object (File)
Thu, Apr 25, 18:25
Unknown Object (File)
Thu, Apr 25, 18:18
Unknown Object (File)
Mon, Apr 22, 11:30
Unknown Object (File)
Mon, Apr 22, 03:17
Unknown Object (File)
Mon, Apr 22, 02:24
Unknown Object (File)
Sun, Apr 21, 21:13
Unknown Object (File)
Mon, Apr 8, 00:43
Tokens
"Love" token, awarded by valerio.bozzolan.

Details

Summary

Update PhutilCowsay.php to work for small cows.

In doing so, we also simplify the code to just use multiline regexps rather than trying to parse a line at a time.

________________ < What about me? > ---------------- \ ,__, \ (oo)____ (__) )\ ||--|| *
Test Plan

Check cowsay works for the built in non-perl cow:

cowsay(cow='companion'){{{Built in}}}

Test all the perl cows:

cowsay(cow='bunny'){{{Testing bunny}}}
cowsay(cow='cower'){{{Testing cower}}}
cowsay(cow='daemon'){{{Testing daemon}}}
cowsay(cow='default'){{{Testing default}}}
cowsay(cow='dragon-and-cow'){{{Testing dragon-and-cow}}}
cowsay(cow='dragon'){{{Testing dragon}}}
cowsay(cow='elephant'){{{Testing elephant}}}
cowsay(cow='eyes'){{{Testing eyes}}}
cowsay(cow='flaming-sheep'){{{Testing flaming-sheep}}}
cowsay(cow='head-in'){{{Testing head-in}}}
cowsay(cow='kitty'){{{Testing kitty}}}
cowsay(cow='koala'){{{Testing koala}}}
cowsay(cow='meow'){{{Testing meow}}}
cowsay(cow='moofasa'){{{Testing moofasa}}}
cowsay(cow='moose'){{{Testing moose}}}
cowsay(cow='mutilated'){{{Testing mutilated}}}
cowsay(cow='satanic'){{{Testing satanic}}}
cowsay(cow='sheep'){{{Testing sheep}}}
cowsay(cow='skeleton'){{{Testing skeleton}}}
cowsay(cow='small'){{{Testing small}}}
cowsay(cow='squirrel'){{{Testing squirrel}}}
cowsay(cow='stegosaurus'){{{Testing stegosaurus}}}
cowsay(cow='supermilker'){{{Testing supermilker}}}
cowsay(cow='surgery'){{{Testing surgery}}}
cowsay(cow='turkey'){{{Testing turkey}}}
cowsay(cow='turtle'){{{Testing turtle}}}
cowsay(cow='tux'){{{Testing tux}}}
cowsay(cow='www'){{{Testing www}}}

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Sten requested review of this revision.Sep 12 2023, 16:09
Sten planned changes to this revision.Sep 12 2023, 16:10
src/utils/PhutilCowsay.php
44–59

Maybe just $matches (and comment optional)

src/utils/__tests__/cowsay/cube_perl.test
1–13

✅ I examined all the original perl-script cowfile(s) in /usr/share/cowsay/cows/* and I can confirm that all of them have that damn EOC (end of cow) so I'm OK with updating this test script, since the comment was about testing the "original per-script".

Replace $m with $matches, as per review.

Sten marked an inline comment as done.Oct 11 2023, 07:31

I'm trying to propose two unit tests, but I'm not able to make them "fail" before the change. The unit always says "OK" so I think I'm not useful :( But here as reference:

src/utils/__tests__/cowsay/small.expect
 ________________
< What about me? >
 ----------------
  \   ,__,
   \  (oo)____
      (__)    )\
         ||--|| *
src/utils/__tests__/cowsay/small.test
  $thoughts   ,__,
   $thoughts  ($eyes)____
      (__)    )\
         ||--|| *
~~~~~~~~~~
{
  "text": "What about me?"
}

I'm just using against:

arc unit src/utils/__tests__/PhutilCowsayTestCase.php

Before and after this change.

Any idea why is that unit test always resulting "OK" to me?

Anyway, feel free to adapt and pull this inside your change 👍

smallcow test case added.

Test fails under the previous code with:

$ arc unit
   FAIL  PhutilCowsayTestCase::testCowsay
Assertion failed, expected values to be equal (at PhutilCowsayTestCase.php:57): Cowsay Test Case "small.test"
Expected vs Actual Output Diff
--- Old Value
+++ New Value
@@ -1,7 +1,8 @@
 ' __________________
 < How are my eyes? >
  ------------------
+oo = ".." unless (oo);
        \   ,__,
         \  (oo)____
            (__)    )\
               ||--|| *'

Turns out we can simply copy the perl template file, then add in the seperator and test values.

Sten marked an inline comment as done.Oct 12 2023, 16:13

Any problems with this fix?

So, no PHP changes? Interesting.

Thank you so much :) Tested intensively.

This revision is now accepted and ready to land.Nov 13 2023, 07:46

I'm a bit confused (!) I thought there were no PHP changes on Rev 1414 and I approved it, but instead I now see that we have again PHP changes. It's unclear to me whenever the Diff I approved was "updated to reflect changes", or maybe I'm just totally crazy because of real life. Please share your opinion.

In any case, thanks, and:

whatcouldgowrong