Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: Unit Tests

on:
push:
branches:
- trunk
pull_request:

jobs:
test:
runs-on: ubuntu-latest
strategy:
matrix:
php-version: ['7.4', '8.4']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pragmatic start, although I'm wondering whether we should at least cover one in between, e.g. 8.0?

WordPress even tests all versions, and while that might be excessive for this project, maybe there is a middle ground?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never personally understood the reason for that. In my mind if it's compatible with 8.4 then it's compatible with each step between. I guess an argument could be made that it will show at exactly which version something is breaking... but that feels like a moot point if the goal is to be compatible all the way up to 8.4. We've done it this way in a handful of our plugins for years with no noticeable downside.

That said, it's really not a big deal, so I'm happy to add it if you still feel it's best. 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually what you're saying is right. But there can always be nuances between versions in little things we wouldn't foresee, and for a project as big as WordPress Core, I think it's worth going for the always - in other words, test with every PHP version to do the closest possible in detecting any possible errors, even if unlikely.

This SDK is obviously not the same reach/scale, so no need to do it here, but I do think one version in between would be helpful, also to get a better idea when/where something broke. This will make finding the root and fixing the bug faster for us too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sound reasoning. Thanks for walking me through that. I've resolved this by adding 8.0 in d5bdb61


steps:
- uses: actions/checkout@v4

- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php-version }}
coverage: xdebug

- name: Validate composer.json and composer.lock
run: composer validate --strict

- name: Cache Composer packages
id: composer-cache
uses: actions/cache@v3
with:
path: vendor
key: ${{ runner.os }}-php-${{ matrix.php-version }}-${{ hashFiles('**/composer.lock') }}
restore-keys: |
${{ runner.os }}-php-${{ matrix.php-version }}-

- name: Install dependencies
run: composer install --prefer-dist --no-progress

- name: Run unit tests
run: composer phpunit
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,9 @@ vendor/
CLAUDE.md
.cursor/
GEMINI.md

############
## PHPUnit
############

.phpunit.cache/
30 changes: 30 additions & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.5/phpunit.xsd"
bootstrap="vendor/autoload.php"
cacheResultFile=".phpunit.cache/test-results"
executionOrder="depends,defects"
forceCoversAnnotation="true"
beStrictAboutCoversAnnotation="true"
beStrictAboutOutputDuringTests="true"
beStrictAboutTodoAnnotatedTests="true"
failOnRisky="true"
failOnWarning="true"
verbose="true"
colors="true">
<testsuites>
<testsuite name="unit">
<directory suffix="Test.php">tests/unit</directory>
</testsuite>
</testsuites>

<coverage cacheDirectory=".phpunit.cache/code-coverage"
processUncoveredFiles="true">
<include>
<directory suffix=".php">src</directory>
</include>
<exclude>
<file>src/DummyForAnalysis.php</file>
</exclude>
</coverage>
</phpunit>
256 changes: 256 additions & 0 deletions tests/unit/Common/AbstractEnumTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
<?php

declare(strict_types=1);

namespace WordPress\AiClient\Tests\unit\Common;

use BadMethodCallException;
use InvalidArgumentException;
use PHPUnit\Framework\TestCase;
use RuntimeException;

/**
* @covers \WordPress\AiClient\Common\AbstractEnum
*/
class AbstractEnumTest extends TestCase
{
/**
* Tests that from() creates an enum instance with a valid value.
*/
public function testFromWithValidValue(): void
{
$enum = ValidTestEnum::from('first');
$this->assertInstanceOf(ValidTestEnum::class, $enum);
$this->assertSame('first', $enum->value);
$this->assertSame('FIRST_NAME', $enum->name);
}


/**
* Tests that from() throws an exception for invalid values.
*/
public function testFromWithInvalidValueThrowsException(): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('invalid is not a valid backing value for enum WordPress\AiClient\Tests\unit\Common\ValidTestEnum');
ValidTestEnum::from('invalid');
}

/**
* Tests that tryFrom() returns an enum instance for valid values.
*/
public function testTryFromWithValidValue(): void
{
$enum = ValidTestEnum::tryFrom('first');
$this->assertInstanceOf(ValidTestEnum::class, $enum);
$this->assertSame('first', $enum->value);
}

/**
* Tests that tryFrom() returns null for invalid values.
*/
public function testTryFromWithInvalidValueReturnsNull(): void
{
$enum = ValidTestEnum::tryFrom('invalid');
$this->assertNull($enum);
}

/**
* Tests that cases() returns all enum instances.
*/
public function testCasesReturnsAllEnumInstances(): void
{
$cases = ValidTestEnum::cases();
$this->assertCount(2, $cases);

$values = array_map(fn($case) => $case->value, $cases);
$this->assertContains('first', $values);
$this->assertContains('last', $values);

$names = array_map(fn($case) => $case->name, $cases);
$this->assertContains('FIRST_NAME', $names);
$this->assertContains('LAST_NAME', $names);
}

/**
* Tests that enum instances are singletons.
*/
public function testSingletonBehavior(): void
{
$enum1 = ValidTestEnum::from('first');
$enum2 = ValidTestEnum::from('first');
$enum3 = ValidTestEnum::firstName();

$this->assertSame($enum1, $enum2);
$this->assertSame($enum1, $enum3);
}

/**
* Tests static factory methods for creating enum instances.
*/
public function testStaticFactoryMethods(): void
{
$firstName = ValidTestEnum::firstName();
$this->assertSame('first', $firstName->value);
$this->assertSame('FIRST_NAME', $firstName->name);

$lastName = ValidTestEnum::lastName();
$this->assertSame('last', $lastName->value);
$this->assertSame('LAST_NAME', $lastName->name);
}

/**
* Tests that invalid static methods throw exceptions.
*/
public function testInvalidStaticMethodThrowsException(): void
{
$this->expectException(BadMethodCallException::class);
$this->expectExceptionMessage(
'Method WordPress\AiClient\Tests\unit\Common\ValidTestEnum::invalidMethod does not exist'
);
ValidTestEnum::invalidMethod();
}

/**
* Tests the is* check methods.
*/
public function testIsCheckMethods(): void
{
$enum = ValidTestEnum::firstName();

$this->assertTrue($enum->isFirstName());
$this->assertFalse($enum->isLastName());
}

/**
* Tests that invalid is* methods throw exceptions.
*/
public function testInvalidIsMethodThrowsException(): void
{
$enum = ValidTestEnum::firstName();

$this->expectException(BadMethodCallException::class);
$this->expectExceptionMessage(
'Method WordPress\AiClient\Tests\unit\Common\ValidTestEnum::isInvalidMethod does not exist'
);
$enum->isInvalidMethod();
}

/**
* Tests the equals() method with various values.
*/
public function testEqualsWithSameValue(): void
{
$enum = ValidTestEnum::firstName();

$this->assertTrue($enum->equals('first'));
$this->assertTrue($enum->equals(ValidTestEnum::firstName()));
$this->assertFalse($enum->equals('last'));
$this->assertFalse($enum->equals(ValidTestEnum::lastName()));
}


/**
* Tests the is() method for identity comparison.
*/
public function testIsMethodForIdentityComparison(): void
{
$enum1 = ValidTestEnum::firstName();
$enum2 = ValidTestEnum::firstName();
$enum3 = ValidTestEnum::lastName();

$this->assertTrue($enum1->is($enum2)); // Same instance
$this->assertFalse($enum1->is($enum3)); // Different instance
}

/**
* Tests that getValues() returns all valid enum values.
*/
public function testGetValuesReturnsAllValidValues(): void
{
$values = ValidTestEnum::getValues();

$this->assertSame([
'FIRST_NAME' => 'first',
'LAST_NAME' => 'last',
], $values);
}

/**
* Tests the isValidValue() method.
*/
public function testIsValidValue(): void
{
$this->assertTrue(ValidTestEnum::isValidValue('first'));
$this->assertTrue(ValidTestEnum::isValidValue('last'));

$this->assertFalse(ValidTestEnum::isValidValue('invalid'));
}

/**
* Tests that enum properties are read-only.
*/
public function testPropertiesAreReadOnly(): void
{
$enum = ValidTestEnum::firstName();

$this->expectException(BadMethodCallException::class);
$this->expectExceptionMessage(
'Cannot modify property WordPress\AiClient\Tests\unit\Common\ValidTestEnum::value - enum properties are read-only'
);
$enum->value = 'modified';
}

/**
* Tests that accessing invalid properties throws exceptions.
*/
public function testInvalidPropertyAccessThrowsException(): void
{
$enum = ValidTestEnum::firstName();

$this->expectException(BadMethodCallException::class);
$this->expectExceptionMessage(
'Property WordPress\AiClient\Tests\unit\Common\ValidTestEnum::invalid does not exist'
);
$enum->invalid;
}

/**
* Tests the __toString() method.
*/
public function testToString(): void
{
$stringEnum = ValidTestEnum::firstName();

$this->assertSame('first', (string) $stringEnum);
}

/**
* Tests that invalid constant names throw exceptions.
*/
public function testInvalidConstantNameThrowsException(): void
{
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage(
'Invalid enum constant name "invalid_name" in ' .
'WordPress\AiClient\Tests\unit\Common\InvalidNameTestEnum. Constants must be UPPER_SNAKE_CASE.'
);

InvalidNameTestEnum::cases();
}

/**
* Tests that invalid constant types throw exceptions.
*/
public function testInvalidConstantTypeThrowsException(): void
{
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage(
'Invalid enum value type for constant ' .
'WordPress\AiClient\Tests\unit\Common\InvalidTypeTestEnum::INT_VALUE. ' .
'Only string values are allowed, integer given.'
);

InvalidTypeTestEnum::cases();
}
}
17 changes: 17 additions & 0 deletions tests/unit/Common/InvalidNameTestEnum.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a common pattern in PHP projects to have mock files or utility classes for testing colocated between all the actual tests?

If so, I'd be okay with that, but based on what I'm used to from WordPress, I'd consider this a bit messy. An alternative would be to separate tests/unit/tests from tests/unit/lib or something like that. Then the first would only contain actual tests.

Curious what you think. Have you seen the pattern here in other popular PHP projects?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought about that. I actually meant to circle back, as I don't like colocation because it breaks the 1-to-1 pattern of src to the unit tests. The pattern I've liked is tests/mocks, so I'll start with that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tentatively resolved in fbc4501. Open to your thoughts!


declare(strict_types=1);

namespace WordPress\AiClient\Tests\unit\Common;

use WordPress\AiClient\Common\AbstractEnum;

/**
* Invalid test enum with lowercase constant name.
*/
class InvalidNameTestEnum extends AbstractEnum
{
public const VALID_NAME = 'valid';
// phpcs:ignore Generic.NamingConventions.UpperCaseConstantName.ClassConstantNotUpperCase
public const invalid_name = 'invalid'; // This should cause an exception
}
16 changes: 16 additions & 0 deletions tests/unit/Common/InvalidTypeTestEnum.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace WordPress\AiClient\Tests\unit\Common;

use WordPress\AiClient\Common\AbstractEnum;

/**
* Invalid test enum with float value.
*/
class InvalidTypeTestEnum extends AbstractEnum
{
public const VALID_VALUE = 'valid';
public const INT_VALUE = 42; // This should cause an exception
}
21 changes: 21 additions & 0 deletions tests/unit/Common/ValidTestEnum.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

declare(strict_types=1);

namespace WordPress\AiClient\Tests\unit\Common;

use WordPress\AiClient\Common\AbstractEnum;

/**
* Valid test enum for testing AbstractEnum functionality.
*
* @method static self firstName() Creates an instance for FIRST_NAME.
* @method static self lastName() Creates an instance for LAST_NAME.
* @method bool isFirstName() Checks if the value is FIRST_NAME.
* @method bool isLastName() Checks if the value is LAST_NAME.
*/
class ValidTestEnum extends AbstractEnum
{
public const FIRST_NAME = 'first';
public const LAST_NAME = 'last';
}
Loading