ColourUtil tests and small refactors

src/CliMenuBuilder.php

@@ -199,7 +199,7 @@ public function setExitButtonText(string $exitButtonText) : self
         return $this;
     }
 
-    public function setBackgroundColour($colour, string $fallback = null) : self
+    public function setBackgroundColour(string $colour, string $fallback = null) : self
     {
         $this->style['bg'] = ColourUtil::validateColour(
             $this->terminal,
@@ -210,7 +210,7 @@ public function setBackgroundColour($colour, string $fallback = null) : self
         return $this;
     }
 
-    public function setForegroundColour($colour, string $fallback = null) : self
+    public function setForegroundColour(string $colour, string $fallback = null) : self
     {
         $this->style['fg'] = ColourUtil::validateColour(
             $this->terminal,

src/MenuStyle.php

@@ -20,12 +20,12 @@ class MenuStyle
     protected $terminal;
 
     /**
-     * @var int|string
+     * @var string
      */
     protected $fg;
 
     /**
-     * @var int|string
+     * @var string
      */
     protected $bg;
 
@@ -172,8 +172,11 @@ public function __construct(Terminal $terminal = null)
     {
         $this->terminal = $terminal ?: TerminalFactory::fromSystem();
 
-        $this->setFg(static::$defaultStyleValues['fg']);
-        $this->setBg(static::$defaultStyleValues['bg']);
+        $this->fg = static::$defaultStyleValues['fg'];
+        $this->bg = static::$defaultStyleValues['bg'];
+
+        $this->generateColoursSetCode();
+
         $this->setWidth(static::$defaultStyleValues['width']);
         $this->setPadding(static::$defaultStyleValues['padding']);
         $this->setMargin(static::$defaultStyleValues['margin']);
@@ -199,13 +202,13 @@ public function getDisabledItemText(string $text) : string
      */
     private function generateColoursSetCode() : void
     {
-        if (is_string($this->fg)) {
+        if (!ctype_digit($this->fg)) {
             $fgCode = self::$availableForegroundColors[$this->fg];
         } else {
             $fgCode = sprintf("38;5;%s", $this->fg);
         }
 
-        if (is_string($this->bg)) {
+        if (!ctype_digit($this->bg)) {
             $bgCode = self::$availableBackgroundColors[$this->bg];
         } else {
             $bgCode = sprintf("48;5;%s", $this->bg);
@@ -259,7 +262,7 @@ public function getFg()
         return $this->fg;
     }
 
-    public function setFg($fg, string $fallback = null) : self
+    public function setFg(string $fg, string $fallback = null) : self
     {
         $this->fg = ColourUtil::validateColour(
             $this->terminal,
@@ -276,7 +279,7 @@ public function getBg()
         return $this->bg;
     }
 
-    public function setBg($bg, string $fallback = null) : self
+    public function setBg(string $bg, string $fallback = null) : self
     {
         $this->bg = ColourUtil::validateColour(
             $this->terminal,

src/Util/ColourUtil.php

@@ -284,7 +284,7 @@ class ColourUtil
         255 => 'white',
     ];
 
-    public static function getDefaultColoursNames() : array
+    public static function getDefaultColourNames() : array
     {
         return static::$defaultColoursNames;
     }
@@ -296,7 +296,7 @@ public static function getDefaultColoursNames() : array
     public static function map256To8(int $colourCode) : string
     {
         if (!isset(static::$coloursMap[$colourCode])) {
-            throw new \InvalidArgumentException("Invalid colour code");
+            throw new \InvalidArgumentException('Invalid colour code');
         }
 
         return static::$coloursMap[$colourCode];
@@ -306,22 +306,28 @@ public static function map256To8(int $colourCode) : string
      * Check if $colour exists
      * If it's a 256-colours code and $terminal doesn't support it, returns a fallback value
      */
-    public static function validateColour(Terminal $terminal, $colour, string $fallback = null)
+    public static function validateColour(Terminal $terminal, string $colour, string $fallback = null) : string
     {
-        if (is_int($colour)) {
-            if ($colour < 0 || $colour > 255) {
-                throw new \InvalidArgumentException("Invalid colour code");
-            }
-            if ($terminal->getColourSupport() < 256) {
-                if ($fallback !== null) {
-                    Assertion::inArray($fallback, static::getDefaultColoursNames());
-                    return $fallback;
-                }
-                return static::map256To8($colour);
-            }
-        } else {
-            Assertion::inArray($colour, static::getDefaultColoursNames());
+        if (!ctype_digit($colour)) {
+            return static::validateColourName($colour);
         }
-        return $colour;
+
+        Assertion::between($colour, 0, 255, 'Invalid colour code');
+
+        if ($terminal->getColourSupport() >= 256) {
+            return $colour;
+        }
+
+        if ($fallback !== null) {
+            return static::validateColourName($fallback);
+        }
+
+        return static::map256To8((int) $colour);
+    }
+
+    private static function validateColourName(string $colourName) : string
+    {
+        Assertion::inArray($colourName, static::getDefaultColourNames());
+        return $colourName;
     }
 }

test/CliMenuBuilderTest.php

@@ -145,7 +145,7 @@ public function testSetBgThrowsExceptionWhenColourCodeIsNotInRange() : void
 
         $builder = new CliMenuBuilder;
         $builder->setTerminal($terminal);
-        $builder->setBackgroundColour(-5, 'white');
+        $builder->setBackgroundColour(257, 'white');
     }
 
     public function testDisableDefaultItems() : void

test/MenuStyleTest.php

@@ -74,21 +74,6 @@ public function testMenuStyleCanBeInstantiatedByCliMenuBuilder() : void
         static::assertSame(MenuStyle::class, get_class($style));
     }
 
-    public function testAvailableColours() : void
-    {
-        static::assertSame([
-            'black',
-            'red',
-            'green',
-            'yellow',
-            'blue',
-            'magenta',
-            'cyan',
-            'white',
-            'default'
-        ], ColourUtil::getDefaultColoursNames());
-    }
-
     public function testGetColoursSetCode() : void
     {
         static::assertSame("\e[37;44m", $this->getMenuStyle()->getColoursSetCode());
@@ -152,8 +137,8 @@ public function test256ColoursCodes() : void
         $style = $this->getMenuStyle(256);
         $style->setBg(16, 'white');
         $style->setFg(206, 'red');
-        static::assertSame(16, $style->getBg());
-        static::assertSame(206, $style->getFg());
+        static::assertSame('16', $style->getBg());
+        static::assertSame('206', $style->getFg());
         static::assertSame("\033[38;5;206;48;5;16m", $style->getColoursSetCode());
 
         $style = $this->getMenuStyle(8);
@@ -179,7 +164,7 @@ public function testSetBgThrowsExceptionWhenColourCodeIsNotInRange() : void
         $this->expectExceptionMessage('Invalid colour code');
 
         $style = $this->getMenuStyle(256);
-        $style->setBg(-5, 'white');
+        $style->setBg(257, 'white');
     }
 
     public function testGetMarkerReturnsTheCorrectMarkers() : void

test/Util/ColourUtilTest.php

@@ -0,0 +1,136 @@
+<?php
+
+namespace PhpSchool\CliMenuTest\Util;
+
+use PhpSchool\CliMenu\Util\ColourUtil;
+use PhpSchool\Terminal\Terminal;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @author Aydin Hassan <[email protected]>
+ */
+class ColourUtilTest extends TestCase
+{
+    public function testAvailableColours() : void
+    {
+        self::assertSame(
+            [
+                'black',
+                'red',
+                'green',
+                'yellow',
+                'blue',
+                'magenta',
+                'cyan',
+                'white',
+                'default'
+            ],
+            ColourUtil::getDefaultColourNames()
+        );
+    }
+
+    public function testMap256To8ThrowsExceptionIfCodeNotValid() : void
+    {
+        self::expectException(\InvalidArgumentException::class);
+        self::expectExceptionMessage('Invalid colour code');
+
+        ColourUtil::map256To8(512);
+    }
+
+    public function testMap256To8() : void
+    {
+        self::assertEquals('white', ColourUtil::map256To8(255));
+        self::assertEquals('magenta', ColourUtil::map256To8(213));
+        self::assertEquals('yellow', ColourUtil::map256To8(143));
+        self::assertEquals('blue', ColourUtil::map256To8(103));
+        self::assertEquals('green', ColourUtil::map256To8(64));
+    }
+
+    public function testValidateColourThrowsExceptionIfColourNot256AndNot8() : void
+    {
+        self::expectException(\Assert\InvalidArgumentException::class);
+
+        $terminal = $this->createMock(Terminal::class);
+
+        ColourUtil::validateColour($terminal, 'teal');
+    }
+
+    public function testValidateColourThrowsExceptionIfFallbackNotValidWhenTerminalDoesNotSupport256Colours() : void
+    {
+        self::expectException(\Assert\InvalidArgumentException::class);
+
+        $terminal = $this->createMock(Terminal::class);
+        $terminal->expects($this->once())
+            ->method('getColourSupport')
+            ->willReturn(8);
+
+        ColourUtil::validateColour($terminal, 255, 'teal');
+    }
+
+    public function testValidateColourWithFallbackWhenTerminalDoesNotSupport256Colours() : void
+    {
+        $terminal = $this->createMock(Terminal::class);
+        $terminal->expects($this->once())
+            ->method('getColourSupport')
+            ->willReturn(8);
+
+        self::assertEquals('red', ColourUtil::validateColour($terminal, 255, 'red'));
+    }
+
+    public function testValidateColourPicksFallbackFromPreComputedListWhenTerminalDoesNotSupport256Colours() : void
+    {
+        $terminal = $this->createMock(Terminal::class);
+        $terminal->expects($this->once())
+            ->method('getColourSupport')
+            ->willReturn(8);
+
+        self::assertEquals('yellow', ColourUtil::validateColour($terminal, 148));
+    }
+
+    /**
+     * @dataProvider invalidColourCodeProvider
+     */
+    public function testValidateColourThrowsExceptionIfInvalid256ColourCodeUsed(int $colourCode) : void
+    {
+        self::expectException(\Assert\InvalidArgumentException::class);
+
+        ColourUtil::validateColour($this->createMock(Terminal::class), $colourCode);
+    }
+
+    public function invalidColourCodeProvider() : array
+    {
+        return [
+            [-1],
+            [256],
+            [1000],
+        ];
+    }
+
+    /**
+     * @dataProvider validColourCodeProvider
+     */
+    public function testValidateColourWith256ColoursWhenTerminalSupports256Colours(int $colourCode) : void
+    {
+        $terminal = $this->createMock(Terminal::class);
+        $terminal->expects($this->once())
+            ->method('getColourSupport')
+            ->willReturn(256);
+
+        self::assertEquals($colourCode, ColourUtil::validateColour($terminal, $colourCode));
+    }
+
+    public function validColourCodeProvider() : array
+    {
+        return [
+            [0],
+            [255],
+            [1],
+            [100],
+        ];
+    }
+
+    public function testValidateColourWithValid8ColourName() : void
+    {
+        self::assertEquals('red', ColourUtil::validateColour($this->createMock(Terminal::class), 'red'));
+    }
+}