Skip to content

Commit e5dbc75

Browse files
authored
CI: add something to run php tests (#1356)
## Motivation We recently had a contribution to the PHP lib and verifying the change was sound turned out to be a bit of a wild goose chase. ## Solution We already had some tests in place so it felt natural to get the suite running in CI. After getting that going, there were a few warnings and failures that needed fixing up.
2 parents 0f41c2e + c6d6485 commit e5dbc75

File tree

6 files changed

+48
-2
lines changed

6 files changed

+48
-2
lines changed

.github/workflows/php-ci.yml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
name: PHP CI
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
paths:
8+
- 'php/**'
9+
- '.github/workflows/php-ci.yml'
10+
pull_request:
11+
paths:
12+
- 'php/**'
13+
- '.github/workflows/php-ci.yml'
14+
jobs:
15+
build-test:
16+
runs-on: ubuntu-latest
17+
18+
steps:
19+
- uses: actions/checkout@v3
20+
- name: PHPUnit Tests
21+
uses: php-actions/phpunit@v3
22+
with:
23+
version: latest
24+
configuration: "php/phpunit.xml"
25+
test_suffix: "Test.php"

php/init.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@
22

33
require __DIR__ . '/src/Webhook.php';
44
require __DIR__ . '/src/Exception/WebhookVerificationException.php';
5+
require __DIR__ . '/src/Exception/WebhookSigningException.php';

php/phpunit.xml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<phpunit bootstrap="tests/bootstrap.php">
2+
<testsuites>
3+
<testsuite name="unit">
4+
<directory>tests/</directory>
5+
</testsuite>
6+
</testsuites>
7+
</phpunit>

php/src/Webhook.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ public function verify($payload, $headers)
7070

7171
public function sign($msgId, $timestamp, $payload)
7272
{
73-
$is_positive_integer = is_numeric($timestamp) && (int) $timestamp == $timestamp && (int) $timestamp > 0;
74-
if (!$is_positive_integer) {
73+
if (!self::isPositiveInteger($timestamp)) {
7574
throw new Exception\WebhookSigningException("Invalid timestamp");
7675
}
7776
$toSign = "{$msgId}.{$timestamp}.{$payload}";
@@ -97,4 +96,9 @@ private function verifyTimestamp($timestampHeader)
9796
}
9897
return $timestamp;
9998
}
99+
100+
private function isPositiveInteger($v)
101+
{
102+
return is_numeric($v) && !is_float($v + 0) && (int) $v == $v && (int) $v > 0;
103+
}
100104
}

php/tests/WebhookTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ public function testNewTimestampThrowsException()
112112

113113
public function testMultiSigPayloadIsValid()
114114
{
115+
// We're checking that `verify()` doesn't throw an exception.
116+
// It doesn't return anything we can assert about.
117+
$this->expectNotToPerformAssertions();
118+
115119
$testPayload = new TestPayload(time());
116120
$sigs = [
117121
"v1,Ceo5qEr07ixe2NLpvHk3FH9bwy/WavXrAFQ/9tdO6mc=",
@@ -127,6 +131,10 @@ public function testMultiSigPayloadIsValid()
127131

128132
public function testSignatureVerificationWithAndWithoutPrefix()
129133
{
134+
// We're checking that `verify()` doesn't throw an exception.
135+
// It doesn't return anything we can assert about.
136+
$this->expectNotToPerformAssertions();
137+
130138
$testPayload = new TestPayload(time());
131139

132140
$wh = new \Svix\Webhook($testPayload->secret);

php/tests/bootstrap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
<?php
22

33
require_once __DIR__ . '/../init.php';
4+
require_once __DIR__ . '/TestPayload.php';

0 commit comments

Comments
 (0)