Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
86 changes: 86 additions & 0 deletions .github/workflows/phpunit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
name: PHPUnit

on:
push:
branches: [ master ]
# Deliberately unfiltered by base branch: the money-path work lands as a stack of
# PRs targeting feature branches, and those are exactly the diffs this suite
# exists to gate. A `branches: [master]` filter would skip every one of them.
pull_request:

# Least privilege: this job only needs to read the checkout. Without an explicit
# block the job inherits the repository default GITHUB_TOKEN scope, which on many
# repos is read/write. Deliberately `pull_request`, not `pull_request_target`, so a
# fork's code never runs with a writable token or access to secrets.
permissions:
contents: read

concurrency:
group: phpunit-${{ github.ref }}
cancel-in-progress: true

jobs:
test:
name: Unit tests (PHP ${{ matrix.php }})
runs-on: ubuntu-latest
timeout-minutes: 15

strategy:
fail-fast: false
matrix:
# One version, and adding a second is not just a matter of listing it.
# Test/Unit/composer.lock pins the tree that was resolved on 8.5, and some
# transitive packages in it require >= 8.4.1 — so an 8.3 leg installing from
# this lock fails on platform requirements, and installing *without* the
# lock would defeat the point of pinning it. A second leg needs its own
# lock (or an older framework pin); until someone has watched that go
# green, listing it would just be a permanently red check.
#
# 8.5 is what Adobe QA and dev-repro/ run, and it is the version this
# suite is verified on. Note README/CLAUDE.md advertise "PHP 8.2+", which
# CI therefore does not cover — reconcile the claim or the coverage.
php: [ '8.5' ]

steps:
- name: Checkout repository
uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4
with:
persist-credentials: false

- name: Set up PHP
uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2
with:
php-version: ${{ matrix.php }}
tools: composer:2.9

# Cache Composer's download cache, not vendor/: a restored vendor/ is code
# that gets autoloaded and executed, and with an immutable key it would
# outlive any upstream fix. The download cache is content-addressed and the
# lock is what guarantees which packages get installed.
- name: Cache Composer downloads
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
with:
path: ~/.composer/cache
key: ${{ runner.os }}-php${{ matrix.php }}-composer-${{ hashFiles('Test/Unit/composer.lock') }}
restore-keys: |
${{ runner.os }}-php${{ matrix.php }}-composer-

# Test/Unit/composer.json + .lock are a CI-only manifest resolving against
# mirror.mage-os.org (no Adobe auth needed). They must never be merged into
# the shipped root composer.json, whose require block is deliberately empty:
# the committed root composer.lock has a stale content-hash, so adding
# require-dev there makes composer install refuse, and the regenerated lock
# would ship in the Marketplace zip.
#
# --no-plugins/--no-scripts: nothing about mocking classes needs install-time
# code execution, and the tree would otherwise run a Composer plugin fetched
# from a third-party mirror. Verified: the suite passes without them.
- name: Install test dependencies
working-directory: Test/Unit
run: composer install --no-interaction --no-progress --no-plugins --no-scripts --prefer-dist

# Config lives in the root phpunit.xml so CI and a local run share one
# definition; it excludes Test/Unit/vendor from discovery, because
# magento/framework ships its own *Test.php files which fatal when loaded.
- name: Run PHPUnit
run: Test/Unit/vendor/bin/phpunit -c phpunit.xml --no-coverage
15 changes: 15 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,21 @@
composer.phar
/vendor/

# CI-only unit-test dependencies (Test/Unit/composer.json) — never shipped.
# Test/Unit/composer.lock IS committed on purpose: it pins all 184 packages so the
# gate resolves the same set every run. The reasons for not committing the *root*
# lock do not apply here — build-adobe-zip.sh excludes Test/Unit/*, so this lock
# ships nothing and constrains no merchant.
Test/Unit/vendor/
Test/Unit/.phpunit.cache/

# Composer credentials. Composer reads auth.json from the project directory, so
# Test/Unit/ is where a real Adobe key would actually be placed. Neither belongs
# in a public repo. (The tracked root auth.json holds placeholders and is filed
# for removal separately.)
auth.json
Test/Unit/auth.json

# Random files
*~
\#*
Expand Down
25 changes: 21 additions & 4 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
- **PHP namespace**: `Pstk\Paystack`
- **Composer package**: `pstk/paystack-magento2-module`
- **Magento payment method code**: `pstk_paystack` (constant `Pstk\Paystack\Model\Payment\Paystack::CODE`)
- **Requires**: Magento 2.4.x, PHP 8.2+ (this is the supported target, but note `composer.json` has an empty `require: {}` — these constraints are **not** enforced by Composer)
- **Requires**: Magento 2.4.x, PHP 8.2+ per README — but treat that claim as **unverified**: `magento/framework 103.0.9` (the 2.4.9 line) requires `~8.3.0||~8.4.0||~8.5.0`, so PHP 8.2 cannot even install it, and CI covers 8.5 only. `composer.json` has an empty `require: {}`, so none of this is enforced by Composer.

## Build

Expand All @@ -18,7 +18,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
# Output: pstk-paystack-magento2-module-<version>.zip
```

`build-adobe-zip.sh` reads the version from `composer.json` and always rebuilds from scratch (removes any stale zip first). Its exclusion list is `.git*`, `.DS_Store`, `.claude/`, `dev/`, `dev-ee/`, `dev-repro/`, `marketplace/`, `vendor/`, `.env`, `auth.json`, `CLAUDE.md`, `docs/`, `graphify-out/`, `node_modules/`, the build script itself, and prior `*.zip` builds — so `CLAUDE.md`, internal QA artifacts, and tooling caches never ship to Marketplace.
`build-adobe-zip.sh` reads the version from `composer.json` and always rebuilds from scratch (removes any stale zip first). Its exclusion list is `.git*`, `.DS_Store`, `.claude/`, `dev/`, `dev-ee/`, `dev-repro/`, `marketplace/`, `vendor/`, `.env`, `auth.json`, `CLAUDE.md`, `docs/`, `graphify-out/`, `node_modules/`, `phpunit.xml`, `Test/Unit/`, the build script itself, and prior `*.zip` builds — so `CLAUDE.md`, internal QA artifacts, and tooling caches never ship to Marketplace.

> ⚠️ **Anything added to the repo root after the build script was written must be added to its exclusion list explicitly.** This has already gone wrong once: `dev-ee/` was created after the script and had to be retro-fitted before a release could ship without bundling the entire EE harness. When you add a new top-level directory that is not package content, add its `-x` line **in the same commit**.

Expand Down Expand Up @@ -76,9 +76,26 @@ vendor/bin/mftf run:test PaystackPaymentConfigAvailableTest
vendor/bin/mftf run:test StorefrontPaystackCheckoutRendersTest
```

Current tests (`Test/Mftf/Test/`): `PaystackPaymentConfigAvailableTest.xml` and `StorefrontPaystackCheckoutRendersTest.xml`, backed by the page object `Test/Mftf/Page/AdminPaymentConfigPage.xml`. `Test/Mftf/Suite/` exists but is **empty** — there are no suites, so `vendor/bin/mftf run:suite` has nothing to run.
Current tests (`Test/Mftf/Test/`): `PaystackPaymentConfigAvailableTest.xml` and `StorefrontPaystackCheckoutRendersTest.xml`, backed by the page object `Test/Mftf/Page/PaystackPaymentConfigPage.xml`. `Test/Mftf/Suite/` exists but is **empty** — there are no suites, so `vendor/bin/mftf run:suite` has nothing to run.

There are no unit tests — the test suite is entirely MFTF (browser-level functional tests). There is no configured linter or static-analysis tooling (no PHPCS/PHPStan config, no composer `scripts`); match the surrounding code style by hand. The only CI is `.github/workflows/codeql-analysis.yml` (CodeQL security scanning).
### Unit tests

There are 98 PHPUnit tests in `Test/Unit/`, and they are **not runnable from a fresh checkout** — the shipped `composer.json` has an empty `require` block and no `require-dev` on purpose. The test dependencies live in a CI-only manifest:

```bash
cd Test/Unit && composer install # 184 packages, pinned by the committed lock
cd - && Test/Unit/vendor/bin/phpunit -c phpunit.xml --no-coverage
```

Do **not** move those deps into the root `composer.json`: the committed root `composer.lock` has a stale content-hash (it locks `yabacon/paystack-php`, a package absent from `require`), so `composer install` would refuse until regenerated — and the regenerated lock ships in the Marketplace zip. 3.0.10 passed Adobe review with an empty `require`; keep that surface untouched.

`phpunit.xml` at the repo root is the single config shared by CI and local runs. It bootstraps `Test/Unit/vendor/autoload.php` and excludes `Test/Unit/vendor` from discovery, because `magento/framework` ships its own `*Test.php` files that fatal when loaded. Its cache lives in `Test/Unit/.phpunit.cache` so it cannot leak into the package.

`dev/docker-compose.yml` masks `Test/Unit/vendor` with an anonymous volume — the repo is bind-mounted as a Magento module, and a second `magento/framework` inside it would be scanned by `setup:di:compile`. **`dev-repro/` needs the same line but is gitignored and untracked**, so that fix is local-only; anyone recreating `dev-repro/` must re-add it.

There is no configured linter or static-analysis tooling (no PHPCS/PHPStan config, no composer `scripts`); match the surrounding code style by hand.

CI is `.github/workflows/phpunit.yml` (unit tests, PHP 8.5) and `.github/workflows/codeql-analysis.yml` (CodeQL — note it scans **JavaScript only**, so the PHP money path gets no static analysis, and it still pins retired action/CodeQL v1 versions).

The `docs/` directory (gitignored, never shipped) holds local MFTF Allure report artifacts (`mftfmagento/`, `mftfvendor/`) plus the `EE-NO-MODULE-BASELINE.md` analysis — it is not module code.

Expand Down
29 changes: 20 additions & 9 deletions Test/Unit/Observer/ObserverAfterPaymentVerifyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,19 @@ public function testPendingOrderTransitionsToProcessing(): void
->method('send')
->with($order, true);

$eventObserver = $this->createMock(Observer::class);
$eventObserver->method('getPaystackOrder')->willReturn($order);
$eventObserver = new Observer(['paystack_order' => $order]);

$this->observer->execute($eventObserver);
}

/**
* NOTE for the verification-gate work: this asserts the *status string* gate
* that the observer currently uses, which is itself a known defect — a merchant
* who assigns a custom default status to state New (e.g. `awaiting_payment`)
* gets orders whose status is not the literal 'pending', so the observer no-ops
* on every verified payment. When that is fixed to gate on state, this test is
* expected to change; that is a planned correction, not a green test being bent.
*/
public function testNonPendingOrderIsNotUpdated(): void
{
$order = $this->createMock(Order::class);
Expand All @@ -68,8 +75,7 @@ public function testNonPendingOrderIsNotUpdated(): void
$order->expects($this->never())->method('setState');
$order->expects($this->never())->method('save');

$eventObserver = $this->createMock(Observer::class);
$eventObserver->method('getPaystackOrder')->willReturn($order);
$eventObserver = new Observer(['paystack_order' => $order]);

$this->observer->execute($eventObserver);
}
Expand All @@ -88,19 +94,24 @@ public function testEmailSendingFailureDoesNotAffectOrderStatus(): void
$this->orderSender->method('send')
->willThrowException(new \Exception('SMTP failure'));

$eventObserver = $this->createMock(Observer::class);
$eventObserver->method('getPaystackOrder')->willReturn($order);
$eventObserver = new Observer(['paystack_order' => $order]);

// Should not throw
$this->observer->execute($eventObserver);
}

public function testNullOrderDoesNotCrash(): void
{
$eventObserver = $this->createMock(Observer::class);
$eventObserver->method('getPaystackOrder')->willReturn(null);
// This case genuinely only guarantees "does not throw": remove the `$order &&`
// guard from the production code and it dereferences null at getStatus(),
// failing here on that Error before the never() below could be evaluated.
// The never() is therefore a smoke check, not the assertion doing the work.
// The distinguishing negative case — a real order that must NOT advance — is
// testNonPendingOrderIsNotUpdated above.
$this->orderSender->expects($this->never())->method('send');

$eventObserver = new Observer(['paystack_order' => null]);

// Should not throw
$this->observer->execute($eventObserver);
}
}
24 changes: 12 additions & 12 deletions Test/Unit/Observer/ObserverBeforeSalesOrderPlaceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,9 @@ public function testPaystackOrderSuppressesEmail(): void
->method('setCustomerNoteNotify')
->with(false);

$event = $this->createMock(Event::class);
$event->method('getOrder')->willReturn($order);
$event = new Event(['order' => $order]);

$eventObserver = $this->createMock(Observer::class);
$eventObserver->method('getEvent')->willReturn($event);
$eventObserver = new Observer(['event' => $event]);

$this->observer->execute($eventObserver);
}
Expand All @@ -54,11 +52,9 @@ public function testNonPaystackOrderDoesNotSuppressEmail(): void
$order->method('getPayment')->willReturn($payment);
$order->expects($this->never())->method('setCanSendNewEmailFlag');

$event = $this->createMock(Event::class);
$event->method('getOrder')->willReturn($order);
$event = new Event(['order' => $order]);

$eventObserver = $this->createMock(Observer::class);
$eventObserver->method('getEvent')->willReturn($event);
$eventObserver = new Observer(['event' => $event]);

$this->observer->execute($eventObserver);
}
Expand All @@ -67,12 +63,16 @@ public function testNullPaymentDoesNotCrash(): void
{
$order = $this->createMock(Order::class);
$order->method('getPayment')->willReturn(null);
// This case genuinely only guarantees "does not throw": remove the
// `$order->getPayment() &&` guard from the production code and null->getMethod()
// throws before setCanSendNewEmailFlag is reachable, so the test fails on that
// Error rather than on the never(). The distinguishing negative case — a real
// payment for a different method — is testNonPaystackOrderDoesNotSuppressEmail.
$order->expects($this->never())->method('setCanSendNewEmailFlag');

$event = $this->createMock(Event::class);
$event->method('getOrder')->willReturn($order);
$event = new Event(['order' => $order]);

$eventObserver = $this->createMock(Observer::class);
$eventObserver->method('getEvent')->willReturn($event);
$eventObserver = new Observer(['event' => $event]);

$this->observer->execute($eventObserver);
}
Expand Down
27 changes: 27 additions & 0 deletions Test/Unit/composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"name": "pstk/paystack-magento2-module-tests",
"description": "CI-only manifest for running Test/Unit. Never shipped (build-adobe-zip.sh excludes Test/Unit/*). Do NOT move these deps into the root composer.json: its require block must stay empty, and the committed root composer.lock has a stale content-hash that would make composer install refuse.",
"version": "1.0.0",
"repositories": [
{
"type": "composer",
"url": "https://mirror.mage-os.org/"
}
],
"require": {
"phpunit/phpunit": "10.5.64",
"magento/framework": "103.0.9",
"magento/module-sales": "103.0.9",
"magento/module-payment": "100.4.9",
"magento/module-quote": "101.2.9",
"magento/module-store": "101.1.9",
"magento/module-checkout": "100.4.9",
"magento/module-customer": "103.0.9"
},
"prefer-stable": true,
"autoload": {
"psr-4": {
"Pstk\\Paystack\\": "../../"
}
}
}
Loading
Loading