Skip to content

Commit

Permalink
CI: add something to run php tests (#1356)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
svix-onelson authored Jul 5, 2024
2 parents 0f41c2e + c6d6485 commit e5dbc75
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 2 deletions.
25 changes: 25 additions & 0 deletions .github/workflows/php-ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
name: PHP CI

on:
push:
branches:
- main
paths:
- 'php/**'
- '.github/workflows/php-ci.yml'
pull_request:
paths:
- 'php/**'
- '.github/workflows/php-ci.yml'
jobs:
build-test:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- name: PHPUnit Tests
uses: php-actions/phpunit@v3
with:
version: latest
configuration: "php/phpunit.xml"
test_suffix: "Test.php"
1 change: 1 addition & 0 deletions php/init.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@

require __DIR__ . '/src/Webhook.php';
require __DIR__ . '/src/Exception/WebhookVerificationException.php';
require __DIR__ . '/src/Exception/WebhookSigningException.php';
7 changes: 7 additions & 0 deletions php/phpunit.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<phpunit bootstrap="tests/bootstrap.php">
<testsuites>
<testsuite name="unit">
<directory>tests/</directory>
</testsuite>
</testsuites>
</phpunit>
8 changes: 6 additions & 2 deletions php/src/Webhook.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ public function verify($payload, $headers)

public function sign($msgId, $timestamp, $payload)
{
$is_positive_integer = is_numeric($timestamp) && (int) $timestamp == $timestamp && (int) $timestamp > 0;
if (!$is_positive_integer) {
if (!self::isPositiveInteger($timestamp)) {
throw new Exception\WebhookSigningException("Invalid timestamp");
}
$toSign = "{$msgId}.{$timestamp}.{$payload}";
Expand All @@ -97,4 +96,9 @@ private function verifyTimestamp($timestampHeader)
}
return $timestamp;
}

private function isPositiveInteger($v)
{
return is_numeric($v) && !is_float($v + 0) && (int) $v == $v && (int) $v > 0;
}
}
8 changes: 8 additions & 0 deletions php/tests/WebhookTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ public function testNewTimestampThrowsException()

public function testMultiSigPayloadIsValid()
{
// We're checking that `verify()` doesn't throw an exception.
// It doesn't return anything we can assert about.
$this->expectNotToPerformAssertions();

$testPayload = new TestPayload(time());
$sigs = [
"v1,Ceo5qEr07ixe2NLpvHk3FH9bwy/WavXrAFQ/9tdO6mc=",
Expand All @@ -127,6 +131,10 @@ public function testMultiSigPayloadIsValid()

public function testSignatureVerificationWithAndWithoutPrefix()
{
// We're checking that `verify()` doesn't throw an exception.
// It doesn't return anything we can assert about.
$this->expectNotToPerformAssertions();

$testPayload = new TestPayload(time());

$wh = new \Svix\Webhook($testPayload->secret);
Expand Down
1 change: 1 addition & 0 deletions php/tests/bootstrap.php
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<?php

require_once __DIR__ . '/../init.php';
require_once __DIR__ . '/TestPayload.php';

0 comments on commit e5dbc75

Please sign in to comment.