Skip to content

Commit

Permalink
test: coverage improvements and chores (#65)
Browse files Browse the repository at this point in the history
## This PR

- test: add shared base hook classes cases
- fix!: rename finallyAfter to finally in AbstractHook
- refactor: default to set NoOpProvider
- chore: cleanup commented code for $_SESSION
- test: can get version from Client
- feat: hook execution reversal
- test: cases for NoOpClient
- test: implement decapitalize unit tests

### Related Issues

Fixes #54
Fixes #64
Fixes #55
Fixes #63 
Fixes #57 
Fixes #62 
Fixes #59 
Fixes #60

Signed-off-by: Tom Carrio <tom@carrio.dev>
  • Loading branch information
tcarrio authored Jan 2, 2023
1 parent 446648a commit 5dc93aa
Show file tree
Hide file tree
Showing 7 changed files with 343 additions and 28 deletions.
22 changes: 3 additions & 19 deletions src/OpenFeatureAPI.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,13 @@
use function array_merge;
use function is_null;

class OpenFeatureAPI implements API, LoggerAwareInterface
final class OpenFeatureAPI implements API, LoggerAwareInterface
{
use LoggerAwareTrait;

private static ?OpenFeatureAPI $instance = null;

//// TODO: Support global using $_SESSION?
// private const GLOBAL_OPEN_FEATURE_KEY = '__OPENFEATURE_INSTANCE_ID__';

private ?Provider $provider = null;
private Provider $provider;

/** @var Hook[] $hooks */
private array $hooks = [];
Expand All @@ -43,15 +40,6 @@ class OpenFeatureAPI implements API, LoggerAwareInterface
*/
public static function getInstance(): API
{
//// TODO: Support global using $_SESSION?
// if (isset($_SESSION)) {
// if (is_null($_SESSION[self::GLOBAL_OPEN_FEATURE_KEY])) {
// $_SESSION[self::GLOBAL_OPEN_FEATURE_KEY] = new self();
// }

// return $_SESSION[self::GLOBAL_OPEN_FEATURE_KEY];
// }

if (is_null(self::$instance)) {
self::$instance = new self();
}
Expand All @@ -70,15 +58,11 @@ public static function getInstance(): API
*/
private function __construct()
{
// no-op
$this->provider = new NoOpProvider();
}

public function getProvider(): Provider
{
if (!$this->provider) {
return new NoOpProvider();
}

return $this->provider;
}

Expand Down
10 changes: 3 additions & 7 deletions src/OpenFeatureClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
use Throwable;

use function array_merge;
use function array_reverse;
use function sprintf;

class OpenFeatureClient implements Client, LoggerAwareInterface
Expand Down Expand Up @@ -331,13 +332,8 @@ private function evaluateFlag(
$options->getHooks(),
$provider->getHooks(),
);
// TODO: Should we do a complete reversal of $mergedBeforeHooks instead?
$mergedRemainingHooks = array_merge(
$provider->getHooks(),
$options->getHooks(),
$this->getHooks(),
$api->getHooks(),
);

$mergedRemainingHooks = array_reverse(array_merge([], $mergedBeforeHooks));

try {
$contextFromBeforeHook = $hookExecutor->beforeHooks($flagValueType, $hookContext, $mergedBeforeHooks, $hookHints);
Expand Down
2 changes: 1 addition & 1 deletion src/implementation/hooks/AbstractHook.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ abstract public function after(HookContext $context, ResolutionDetails $details,

abstract public function error(HookContext $context, Throwable $error, HookHints $hints): void;

abstract public function finallyAfter(HookContext $context, HookHints $hints): void;
abstract public function finally(HookContext $context, HookHints $hints): void;

abstract public function supportsFlagValueType(string $flagValueType): bool;
}
241 changes: 241 additions & 0 deletions tests/unit/BaseHooksTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
<?php

declare(strict_types=1);

namespace OpenFeature\Test\unit;

use OpenFeature\Test\TestCase;
use OpenFeature\implementation\hooks\BooleanHook;
use OpenFeature\implementation\hooks\FloatHook;
use OpenFeature\implementation\hooks\IntegerHook;
use OpenFeature\implementation\hooks\ObjectHook;
use OpenFeature\implementation\hooks\StringHook;
use OpenFeature\interfaces\flags\EvaluationContext;
use OpenFeature\interfaces\flags\FlagValueType;
use OpenFeature\interfaces\hooks\HookContext;
use OpenFeature\interfaces\hooks\HookHints;
use OpenFeature\interfaces\provider\ResolutionDetails;
use Throwable;

class BaseHooksTest extends TestCase
{
/**
* @dataProvider dataBooleanHook
*/
public function testBooleanHook(string $flagValueType, bool $supportsFlagValueType): void
{
$testBooleanHook = new class extends BooleanHook {
public function before(HookContext $context, HookHints $hints): ?EvaluationContext
{
return null;
}

public function after(HookContext $context, ResolutionDetails $details, HookHints $hints): void
{
// no-op
}

public function error(HookContext $context, Throwable $error, HookHints $hints): void
{
// no-op
}

public function finally(HookContext $context, HookHints $hints): void
{
// no-op
}
};

$this->assertEquals($supportsFlagValueType, $testBooleanHook->supportsFlagValueType($flagValueType));
}

/**
* @return Array<Array<mixed>>
*/
public function dataBooleanHook(): array
{
return [
[FlagValueType::BOOLEAN, true],
[FlagValueType::FLOAT, false],
[FlagValueType::INTEGER, false],
[FlagValueType::OBJECT, false],
[FlagValueType::STRING, false],
];
}

/**
* @dataProvider dataFloatHook
*/
public function testFloatHook(string $flagValueType, bool $supportsFlagValueType): void
{
$testFloatHook = new class extends FloatHook {
public function before(HookContext $context, HookHints $hints): ?EvaluationContext
{
return null;
}

public function after(HookContext $context, ResolutionDetails $details, HookHints $hints): void
{
// no-op
}

public function error(HookContext $context, Throwable $error, HookHints $hints): void
{
// no-op
}

public function finally(HookContext $context, HookHints $hints): void
{
// no-op
}
};

$this->assertEquals($supportsFlagValueType, $testFloatHook->supportsFlagValueType($flagValueType));
}

/**
* @return Array<Array<mixed>>
*/
public function dataFloatHook(): array
{
return [
[FlagValueType::BOOLEAN, false],
[FlagValueType::FLOAT, true],
[FlagValueType::INTEGER, false],
[FlagValueType::OBJECT, false],
[FlagValueType::STRING, false],
];
}

/**
* @dataProvider dataIntegerHook
*/
public function testIntegerHook(string $flagValueType, bool $supportsFlagValueType): void
{
$testIntegerHook = new class extends IntegerHook {
public function before(HookContext $context, HookHints $hints): ?EvaluationContext
{
return null;
}

public function after(HookContext $context, ResolutionDetails $details, HookHints $hints): void
{
// no-op
}

public function error(HookContext $context, Throwable $error, HookHints $hints): void
{
// no-op
}

public function finally(HookContext $context, HookHints $hints): void
{
// no-op
}
};

$this->assertEquals($supportsFlagValueType, $testIntegerHook->supportsFlagValueType($flagValueType));
}

/**
* @return Array<Array<mixed>>
*/
public function dataIntegerHook(): array
{
return [
[FlagValueType::BOOLEAN, false],
[FlagValueType::FLOAT, false],
[FlagValueType::INTEGER, true],
[FlagValueType::OBJECT, false],
[FlagValueType::STRING, false],
];
}

/**
* @dataProvider dataObjectHook
*/
public function testObjectHook(string $flagValueType, bool $supportsFlagValueType): void
{
$testObjectHook = new class extends ObjectHook {
public function before(HookContext $context, HookHints $hints): ?EvaluationContext
{
return null;
}

public function after(HookContext $context, ResolutionDetails $details, HookHints $hints): void
{
// no-op
}

public function error(HookContext $context, Throwable $error, HookHints $hints): void
{
// no-op
}

public function finally(HookContext $context, HookHints $hints): void
{
// no-op
}
};

$this->assertEquals($supportsFlagValueType, $testObjectHook->supportsFlagValueType($flagValueType));
}

/**
* @return Array<Array<mixed>>
*/
public function dataObjectHook(): array
{
return [
[FlagValueType::BOOLEAN, false],
[FlagValueType::FLOAT, false],
[FlagValueType::INTEGER, false],
[FlagValueType::OBJECT, true],
[FlagValueType::STRING, false],
];
}

/**
* @dataProvider dataStringHook
*/
public function testStringHook(string $flagValueType, bool $supportsFlagValueType): void
{
$testStringHook = new class extends StringHook {
public function before(HookContext $context, HookHints $hints): ?EvaluationContext
{
return null;
}

public function after(HookContext $context, ResolutionDetails $details, HookHints $hints): void
{
// no-op
}

public function error(HookContext $context, Throwable $error, HookHints $hints): void
{
// no-op
}

public function finally(HookContext $context, HookHints $hints): void
{
// no-op
}
};

$this->assertEquals($supportsFlagValueType, $testStringHook->supportsFlagValueType($flagValueType));
}

/**
* @return Array<Array<mixed>>
*/
public function dataStringHook(): array
{
return [
[FlagValueType::BOOLEAN, false],
[FlagValueType::FLOAT, false],
[FlagValueType::INTEGER, false],
[FlagValueType::OBJECT, false],
[FlagValueType::STRING, true],
];
}
}
56 changes: 56 additions & 0 deletions tests/unit/NoOpClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
use OpenFeature\implementation\flags\EvaluationContext;
use OpenFeature\implementation\flags\EvaluationDetailsBuilder;
use OpenFeature\implementation\flags\NoOpClient;
use OpenFeature\interfaces\hooks\Hook;
use OpenFeature\interfaces\hooks\HookContext;
use OpenFeature\interfaces\hooks\HookHints;
use OpenFeature\interfaces\provider\ResolutionDetails;
use Throwable;

class NoOpClientTest extends TestCase
{
Expand Down Expand Up @@ -156,4 +161,55 @@ public function testGetHooks(): void

$this->assertEquals($expectedValue, $actualValue);
}

public function testSetEvaluationContext(): void
{
$evaluationContext = new EvaluationContext('override-targeting-key');

$this->client->setEvaluationContext($evaluationContext);

$actualValue = $this->client->getEvaluationContext();

$expectedValue = new EvaluationContext('no-op-targeting-key');

$this->assertEquals($expectedValue, $actualValue);
}

public function testSetHooks(): void
{
$fakeHook = new class implements Hook {
public function before(HookContext $context, HookHints $hints): ?EvaluationContext
{
return null;
}

public function after(HookContext $context, ResolutionDetails $details, HookHints $hints): void
{
// no-op
}

public function error(HookContext $context, Throwable $error, HookHints $hints): void
{
// no-op
}

public function finally(HookContext $context, HookHints $hints): void
{
// no-op
}

public function supportsFlagValueType(string $flagValueType): bool
{
return true;
}
};

$this->client->setHooks([$fakeHook]);

$actualValue = $this->client->getHooks();

$expectedValue = [];

$this->assertEquals($expectedValue, $actualValue);
}
}
Loading

0 comments on commit 5dc93aa

Please sign in to comment.