diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..c780c60 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,20 @@ +version: 2 + +updates: + # Composer bumps arrive as their own pull request, so a dependency moving is + # a release of its own rather than something folded into unrelated work. + - package-ecosystem: composer + directory: / + schedule: + interval: weekly + commit-message: + prefix: fix + prefix-development: chore + include: scope + + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + commit-message: + prefix: ci diff --git a/.github/workflows/pr-title.yml b/.github/workflows/pr-title.yml new file mode 100644 index 0000000..dc0c434 --- /dev/null +++ b/.github/workflows/pr-title.yml @@ -0,0 +1,35 @@ +name: PR Title + +# Merges are squashed with the pull request title as the commit subject, so the +# title is what semantic-release will read. Checking it here is the difference +# between a release that happens and one that silently does not. +on: + pull_request_target: + types: [opened, edited, synchronize, reopened] + +permissions: + pull-requests: read + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: amannn/action-semantic-pull-request@v6 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + types: | + feat + fix + perf + refactor + docs + test + build + ci + chore + revert + requireScope: false + subjectPattern: ^(?![A-Z]).+[^.]$ + subjectPatternError: | + The subject "{subject}" should start lower case and not end with a period. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..e692eae --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,42 @@ +name: Release + +# Every merge to main is a candidate. semantic-release reads the commit subjects +# since the last tag and decides whether there is anything to cut — a docs or +# chore change releases nothing at all. +on: + push: + branches: [main] + +# The tag, the GitHub release, and the CHANGELOG commit all need writing. +permissions: + contents: write + issues: write + pull-requests: write + +concurrency: + group: release + cancel-in-progress: false + +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + with: + # Tags are how semantic-release knows the current version, so the + # whole history has to be there. + fetch-depth: 0 + persist-credentials: false + + - uses: actions/setup-node@v7 + with: + node-version: 22 + + - uses: cycjimmy/semantic-release-action@v6 + with: + extra_plugins: | + @semantic-release/changelog@6 + @semantic-release/git@10 + conventional-changelog-conventionalcommits@8 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml new file mode 100644 index 0000000..e3366ab --- /dev/null +++ b/.github/workflows/static-analysis.yml @@ -0,0 +1,23 @@ +name: Static Analysis + +on: + pull_request: + push: + branches: [main] + +jobs: + phpstan: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - name: Install PHP + uses: shivammathur/setup-php@v2 + with: + php-version: 8.5 + + - name: Install packages + run: composer install --prefer-dist --no-progress + + - name: Run PHPStan + run: composer analyse diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..f7efb72 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,38 @@ +name: Tests + +on: + pull_request: + push: + branches: [main] + +jobs: + tests: + name: PHP ${{ matrix.php }} + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + php: ['8.5'] + experimental: [false] + include: + # 8.6 is still in development and parts of the Tempest dependency tree + # do not install on it yet, so it reports without blocking a merge. + - php: '8.6' + experimental: true + + continue-on-error: ${{ matrix.experimental }} + + steps: + - uses: actions/checkout@v7 + + - name: Install PHP ${{ matrix.php }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + + - name: Install packages + run: composer install --prefer-dist --no-progress + + - name: Run tests + run: composer test diff --git a/.releaserc.json b/.releaserc.json new file mode 100644 index 0000000..8164edd --- /dev/null +++ b/.releaserc.json @@ -0,0 +1,32 @@ +{ + "branches": ["main"], + "tagFormat": "v${version}", + "plugins": [ + [ + "@semantic-release/commit-analyzer", + { + "preset": "conventionalcommits" + } + ], + [ + "@semantic-release/release-notes-generator", + { + "preset": "conventionalcommits" + } + ], + [ + "@semantic-release/changelog", + { + "changelogFile": "CHANGELOG.md" + } + ], + "@semantic-release/github", + [ + "@semantic-release/git", + { + "assets": ["CHANGELOG.md"], + "message": "chore(release): ${nextRelease.version} [skip ci]\n\n${nextRelease.notes}" + } + ] + ] +} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..d78ab02 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,58 @@ +# Contributing + +## Commit messages decide releases + +Merges are squashed, and the pull request title becomes the commit subject on +`main`. That subject is read by semantic-release, which cuts the tag and the +GitHub release — so the title is not a formality, it is the version bump. + +Titles follow [Conventional Commits](https://www.conventionalcommits.org): + +``` +feat(cron): support step values over a range +fix(runner): keep a task that throws in the schedule +docs: explain why both day fields restricted means either +``` + +| Prefix | Release | +| --- | --- | +| `fix`, `perf` | patch — `1.2.3` → `1.2.4` | +| `feat` | minor — `1.2.3` → `1.3.0` | +| `feat!`, or `BREAKING CHANGE:` in the body | major — `1.2.3` → `2.0.0` | +| `docs`, `test`, `refactor`, `build`, `ci`, `chore` | none | + +A pull request whose title does not parse is rejected by a check before it can +be merged, because a subject semantic-release cannot read is a release that +silently never happens. + +Put the reasoning in the pull request body. It becomes the commit body, and it +is the part someone reads in a year when they are trying to work out why. + +## Breaking changes + +Mark them, and say what to do instead: + +``` +feat(task)!: name a method task after its class as well + +BREAKING CHANGE: an unnamed method task is now "Housekeeping::sweep" +rather than "sweep". Pass name: to keep the old one. +``` + +## Before opening a pull request + +```bash +composer test # PHPUnit +composer analyse # PHPStan +``` + +## Testing a scheduler + +The suite drives a fake loop rather than waiting out the schedules it exercises +— an interval of a second is the smallest the attribute allows, and a cron +task's next turn can be an hour away. `FakeLoop` records what was armed and +fires it on demand, so a test can say exactly when a turn happens and assert on +the wait that was chosen. + +Nothing here should need `sleep`. If a test seems to, the thing it is testing +probably wants a seam rather than the test wanting patience. diff --git a/README.md b/README.md index cd1fe0c..9e1eaaf 100644 --- a/README.md +++ b/README.md @@ -1,20 +1,9 @@ -# Tempcord Tasks Plugin +# Tempcord Tasks -Scheduled tasks plugin for the Tempcord Discord bot framework. Provides cron-based and interval-based task scheduling for your Discord bot. - -## Features - -- **Interval-based tasks** - Run tasks at regular intervals (every X seconds) -- **Cron-based tasks** - Schedule tasks using standard cron expressions -- **Task statistics** - Track execution times, success rates, and failures -- **Automatic discovery** - Tasks are auto-discovered via PHP attributes, in your bot or in - any package you install -- **Run on boot** - Optionally run tasks immediately when the bot starts - -## Requirements - -- PHP 8.5 or newer -- `tempcord/framework` 0.7 or newer +Scheduled tasks for the [Tempcord](https://github.com/Tempcord/framework) Discord bot framework: +work the bot does on a timer rather than in reply to anything — sweeping rows that have run +their course, lifting blocks that have expired, polling a service with no gateway event of +its own. ## Installation @@ -22,98 +11,145 @@ Scheduled tasks plugin for the Tempcord Discord bot framework. Provides cron-bas composer require tempcord/tasks ``` -That is the whole setup. Tempest discovers the package, `TasksPlugin` is picked up because -it implements `Tempcord\Plugins\Plugin`, and the scheduler starts with the bot — there is -nothing to register by hand. +## Declaring a task -## Usage - -### Basic Interval Task +On the class, the way every other Tempcord handler is declared: ```php use Tempcord\Plugins\Tasks\Attributes\Task; -class MyTasks +#[Task(interval: 10)] +final readonly class SweepTemporaryMessages { - #[Task(interval: 60)] // Run every 60 seconds - public function myTask(): void + public function __construct(private TempMessages $messages) {} + + public function __invoke(): void { - // Your task logic here + $this->messages->sweep(); } } ``` -### Cron-based Task +Or on a method, when several chores belong together and would only be split across classes +to satisfy the attribute: ```php -use Tempcord\Plugins\Tasks\Attributes\Task; - -class MyTasks +final readonly class Housekeeping { - #[Task(cron: '0 * * * *')] // Run every hour at minute 0 - public function hourlyTask(): void - { - // Your task logic here - } + #[Task(interval: 10)] + public function sweepMessages(): void {} - #[Task(cron: '@daily')] // Run once per day at midnight - public function dailyTask(): void - { - // Your task logic here - } + #[Task(cron: '@daily')] + public function pruneStatistics(): void {} } ``` -### Task Options +That is the whole registration. Tasks are discovered like commands and listeners, are built +by the container so they may take whatever dependencies they need, and are put on the event +loop before the gateway opens. + +## What the plugin guarantees + +A timer is less forgiving than an event listener: it fires again whether or not the last +turn finished or threw, forever. Three things are handled so you do not write them into +every task. + +- **A task that throws is logged and keeps its place.** Without that the exception travels + into the event loop, and the usual result is a cancelled timer and nothing ever swept + again — silently, because the bot carries on answering commands and looks healthy. +- **A task is never started alongside itself.** A turn still running when the next is due is + skipped, and the skip logged. Otherwise a task slower than its own interval makes every + following turn slower until nothing else gets a look in. +- **Each turn runs in a fiber**, so a task may `await` the REST API exactly as a command + handler does. + +An ordinary turn is logged at debug rather than info: a task running every ten seconds would +otherwise write eight thousand lines a day saying nothing happened. + +## Options ```php #[Task( - interval: 300, // Run every 5 minutes - runOnBoot: true, // Run immediately when bot starts - name: 'custom-name', // Custom task name (optional) - enabled: true // Enable/disable the task + interval: 300, // run every this many seconds + cron: '0 * * * *', // or on a cron schedule; the two are mutually exclusive + runOnBoot: true, // also take a turn as the bot starts, rather than waiting out + // the first interval + name: 'custom-name', // what to call it in the logs and in tasks:list + enabled: true, // false leaves it out of the schedule entirely )] -public function myTask(): void -{ - // Task logic -} ``` -### Supported Cron Expressions +A task with neither an interval nor a cron expression, with both, or with an interval under +a second is refused while the bot is starting rather than at the moment it would have run. +So is a cron expression that cannot be read. + +Left unnamed, a task is called after its class — `SweepTemporaryMessages` — or after the +class and method for a method-level task — `Housekeeping::sweepMessages`. The class is part +of it because two classes may well have a `sweep()`, and tasks sharing a name would share +their statistics and could not be cancelled apart. + +### The first turn + +The first turn comes after the interval unless `runOnBoot` says otherwise. A task is a +repeating chore; one-off startup work — reconciling against whatever changed while the bot +was down — usually belongs in a plugin's `boot()`, where its ordering against everything +else is visible. + +## Cron expressions + +Standard five-field format: -Standard 5-field cron format: ``` * * * * * │ │ │ │ │ -│ │ │ │ └─ Day of week (0-6, Sunday = 0) -│ │ │ └─── Month (1-12) -│ │ └───── Day of month (1-31) -│ └─────── Hour (0-23) -└───────── Minute (0-59) +│ │ │ │ └─ day of week (0-6, Sunday = 0) +│ │ │ └─── month (1-12) +│ │ └───── day of month (1-31) +│ └─────── hour (0-23) +└───────── minute (0-59) ``` -Aliases: -- `@yearly` or `@annually` - Run once a year (0 0 1 1 *) -- `@monthly` - Run once a month (0 0 1 * *) -- `@weekly` - Run once a week (0 0 * * 0) -- `@daily` or `@midnight` - Run once a day (0 0 * * *) -- `@hourly` - Run once an hour (0 * * * *) +Wildcards (`*`), lists (`1,15,30`), ranges (`9-17`) and steps (`*/5`, `1-10/2`) are +supported. A step over a range counts from where the range starts, so `1-10/2` is +1,3,5,7,9. + +Aliases: `@yearly` / `@annually`, `@monthly`, `@weekly`, `@daily` / `@midnight`, `@hourly`. + +**Both day fields restricted means either.** `0 0 1 * 1` runs on the first of the month +*and* on every Monday, which is what cron does and what anyone writing it expects. When only +one of the two names particular days, it simply narrows. -## Console Commands +A cron task is armed one turn at a time, for the exact wait until the next matching minute, +and re-armed from the turn just taken. Waking every minute to ask whether it is time yet +drifts, and once the drift crosses a minute boundary a matching minute is stepped over and +the task silently does not run that hour. + +## Console -List all registered tasks: ```bash php tempcord tasks:list ``` +Lists every registered task with its schedule and where it is declared. + +## Reaching the schedule at runtime + +```php +use function Tempcord\Plugins\Tasks\tasks; +use function Tempcord\Plugins\Tasks\cancelTask; +use function Tempcord\Plugins\Tasks\taskStats; + +taskStats()['SweepTemporaryMessages']->getSuccessRate(); +cancelTask('SweepTemporaryMessages'); +``` + +`Registry` is injectable, and is the better way in anything the container builds. + ## Requirements -- PHP 8.5 or higher -- Tempcord Framework ^0.6 -- Ragnarok Fenrir ^1 -- Tempest Console ^2 -- Tempest Core ^2 +- PHP 8.5 +- Tempcord Framework >= 0.10 ## License -MIT License +MIT diff --git a/composer.json b/composer.json index f009b13..453c2cc 100644 --- a/composer.json +++ b/composer.json @@ -1,16 +1,21 @@ { "name": "tempcord/tasks", "description": "Scheduled tasks plugin for Tempcord framework", - "version": "0.7.0", "type": "library", + "license": "MIT", "require": { "php": "^8.5", - "tempcord/framework": "^0.7", - "ragnarok/fenrir": "^1.0", + "tempcord/framework": ">=0.10 <1.0", + "react/event-loop": "^1.3", + "react/async": "^4.0", "tempest/console": "^3.18", - "tempest/core": "^3.18" + "tempest/core": "^3.18", + "tempest/log": "^3.18" + }, + "require-dev": { + "phpunit/phpunit": "^13.0", + "phpstan/phpstan": "^2.0" }, - "license": "MIT", "authors": [ { "name": "CyberWolf.Studio", @@ -43,10 +48,6 @@ "Tempcord\\Plugins\\Tasks\\": "src/" } }, - "require-dev": { - "phpunit/phpunit": "^13.0", - "phpstan/phpstan": "^2.1" - }, "autoload-dev": { "psr-4": { "Tempcord\\Plugins\\Tasks\\Tests\\": "tests/" diff --git a/phpunit.xml b/phpunit.xml index 6df8b57..e2757cf 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -3,13 +3,13 @@ xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd" bootstrap="vendor/autoload.php" colors="true" + cacheDirectory=".phpunit.cache" failOnWarning="true" failOnNotice="true" - failOnDeprecation="true" - cacheDirectory=".phpunit.cache"> + failOnDeprecation="true"> - tests + tests/Unit diff --git a/src/Attributes/Task.php b/src/Attributes/Task.php index bff8a21..dc3a51f 100644 --- a/src/Attributes/Task.php +++ b/src/Attributes/Task.php @@ -6,54 +6,63 @@ use Attribute; use InvalidArgumentException; -use Tempest\Reflection\MethodReflector; /** - * Runs a method on a schedule, either every so many seconds or on a cron - * expression. + * Declares work the bot does on a timer. + * + * Goes on an invokable class, the way every other Tempcord attribute does, or + * on a method when several chores belong together and would only be split + * across classes to satisfy the attribute: + * + * #[Task(interval: 60)] + * final readonly class SweepTemporaryMessages { public function __invoke(): void {} } + * + * final readonly class Housekeeping + * { + * #[Task(interval: 10)] + * public function sweepMessages(): void {} + * + * #[Task(cron: '@daily')] + * public function pruneStatistics(): void {} + * } */ -#[Attribute(Attribute::TARGET_METHOD)] -final class Task +#[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD)] +final readonly class Task { - public ?MethodReflector $reflector = null; - /** - * @param int|null $interval run every this many seconds; mutually exclusive with cron - * @param string|null $cron a cron expression; mutually exclusive with interval - * @param bool $runOnBoot also run once as soon as the bot starts - * @param string|null $name defaults to the method's own name - * @param bool $enabled a disabled task is discovered but never scheduled + * @param int|null $interval run every this many seconds; mutually exclusive with $cron + * @param string|null $cron a five field cron expression, or an alias such as '@daily' + * @param bool $runOnBoot also take a turn as soon as the bot starts, rather than + * waiting out the first interval + * @param string|null $name what to call it in the logs and in tasks:list; defaults to + * the class, or the class and method for a method level task + * @param bool $enabled a task registered but left out of the schedule */ public function __construct( - public readonly ?int $interval = null, - public readonly ?string $cron = null, - public readonly bool $runOnBoot = false, - public readonly ?string $name = null, - public readonly bool $enabled = true, + public ?int $interval = null, + public ?string $cron = null, + public bool $runOnBoot = false, + public ?string $name = null, + public bool $enabled = true, ) { if ($interval === null && $cron === null) { - throw new InvalidArgumentException('Task must have either an interval or cron expression'); + throw new InvalidArgumentException('A task must be given either an interval or a cron expression.'); } if ($interval !== null && $cron !== null) { - throw new InvalidArgumentException('Task cannot have both interval and cron expression'); + throw new InvalidArgumentException('A task cannot be given both an interval and a cron expression.'); } + /* + * A zero or negative interval asks the loop to run the task as fast as + * it can, which starves everything else the bot is doing, including the + * gateway heartbeat. + */ if ($interval !== null && $interval < 1) { - throw new InvalidArgumentException('Task interval must be at least 1 second'); + throw new InvalidArgumentException('A task interval must be at least one second.'); } } - public function setReflector(MethodReflector $reflector): void - { - $this->reflector = $reflector; - } - - public function getName(): string - { - return $this->name ?? $this->reflector?->getName() ?? 'unknown'; - } - public function isInterval(): bool { return $this->interval !== null; @@ -63,36 +72,4 @@ public function isCron(): bool { return $this->cron !== null; } - - public function getScheduleDescription(): string - { - if ($this->interval !== null) { - return $this->formatInterval($this->interval); - } - - return "cron: {$this->cron}"; - } - - private function formatInterval(int $seconds): string - { - if ($seconds < 60) { - return 'every ' . $seconds . ' second' . ($seconds > 1 ? 's' : ''); - } - - if ($seconds < 3600) { - $minutes = intdiv($seconds, 60); - - return 'every ' . $minutes . ' minute' . ($minutes > 1 ? 's' : ''); - } - - if ($seconds < 86400) { - $hours = intdiv($seconds, 3600); - - return 'every ' . $hours . ' hour' . ($hours > 1 ? 's' : ''); - } - - $days = intdiv($seconds, 86400); - - return 'every ' . $days . ' day' . ($days > 1 ? 's' : ''); - } } diff --git a/src/Compiler/TaskCompiler.php b/src/Compiler/TaskCompiler.php new file mode 100644 index 0000000..ea95a37 --- /dev/null +++ b/src/Compiler/TaskCompiler.php @@ -0,0 +1,77 @@ +getReflection()->hasMethod('__invoke')) { + throw new RuntimeException( + 'Class [' . $class->getName() . '] should declare an __invoke method', + ); + } + + return $this->build($task, $class, $class->getMethod('__invoke'), $class->getShortName()); + } + + /** + * A task declared on one method of a class that holds several. + */ + public function compileMethod(ClassReflector $class, MethodReflector $method, Task $task): TaskDefinition + { + return $this->build( + $task, + $class, + $method, + $class->getShortName() . '::' . $method->getName(), + ); + } + + private function build( + Task $task, + ClassReflector $class, + MethodReflector $method, + string $defaultName, + ): TaskDefinition { + /* + * Parsed here rather than when the timer is armed, so an expression + * nobody can read fails while the bot is starting and says which task + * it came from — not four hours later inside a timer callback. + */ + if ($task->cron !== null) { + try { + new CronExpression($task->cron); + } catch (Throwable $throwable) { + throw new RuntimeException( + 'Task [' . $defaultName . '] has an unreadable cron expression: ' . $throwable->getMessage(), + previous: $throwable, + ); + } + } + + return new TaskDefinition( + name: $task->name ?? $defaultName, + handler: $class->getName(), + method: $method, + interval: $task->interval, + cron: $task->cron, + runOnBoot: $task->runOnBoot, + enabled: $task->enabled, + ); + } +} diff --git a/src/ConsoleCommands/TasksListCommand.php b/src/ConsoleCommands/TasksListCommand.php index 5bc24da..0092142 100644 --- a/src/ConsoleCommands/TasksListCommand.php +++ b/src/ConsoleCommands/TasksListCommand.php @@ -11,52 +11,41 @@ final readonly class TasksListCommand { public function __construct( - private Registry $tasksRegistry, - private Console $console + private Registry $registry, + private Console $console, ) {} #[ConsoleCommand(name: 'tasks:list', description: 'List all registered scheduled tasks')] public function __invoke(): void { - $tasks = $this->tasksRegistry; - - if ($tasks->count() === 0) { + if ($this->registry->count() === 0) { $this->console->writeln("No tasks registered"); + return; } $this->console->info('Registered Tasks:'); - foreach ($tasks->getAllTasks() as $index => $task) { + foreach ($this->registry->all() as $task) { $status = $task->enabled ? '✓' : '✗'; - $name = $task->getName(); - $schedule = $task->getScheduleDescription(); $runOnBoot = $task->runOnBoot ? " (runs on boot)" : ''; - $class = $task->reflector?->getDeclaringClass()->getShortName() ?? 'Unknown'; $this->console->writeln(sprintf( ' %s %s - %s%s', $status, - $name, - $schedule, - $runOnBoot + $task->name, + $task->schedule(), + $runOnBoot, )); $this->console->writeln(sprintf( " %s::%s()", - $class, - $task->reflector?->getName() ?? 'unknown' + $task->handler, + $task->method->getName(), )); - - if ($index < $tasks->count() - 1) { - $this->console->writeln(''); - } } $this->console->writeln(''); - $this->console->writeln(sprintf( - 'Total: %d task(s)', - $tasks->count() - )); + $this->console->writeln(sprintf('Total: %d task(s)', $this->registry->count())); } } diff --git a/src/Definitions/TaskDefinition.php b/src/Definitions/TaskDefinition.php new file mode 100644 index 0000000..8c48522 --- /dev/null +++ b/src/Definitions/TaskDefinition.php @@ -0,0 +1,62 @@ +interval !== null; + } + + public function isCron(): bool + { + return $this->cron !== null; + } + + /** + * How the schedule reads in the logs and in tasks:list. + */ + public function schedule(): string + { + if ($this->cron !== null) { + return 'cron: ' . $this->cron; + } + + return 'every ' . $this->humanInterval((int) $this->interval); + } + + private function humanInterval(int $seconds): string + { + [$size, $unit] = match (true) { + $seconds < 60 => [$seconds, 'second'], + $seconds < 3600 => [intdiv($seconds, 60), 'minute'], + $seconds < 86400 => [intdiv($seconds, 3600), 'hour'], + default => [intdiv($seconds, 86400), 'day'], + }; + + return $size . ' ' . $unit . ($size === 1 ? '' : 's'); + } +} diff --git a/src/Discoveries/TasksDiscovery.php b/src/Discoveries/TasksDiscovery.php index 574830a..16df650 100644 --- a/src/Discoveries/TasksDiscovery.php +++ b/src/Discoveries/TasksDiscovery.php @@ -5,45 +5,39 @@ namespace Tempcord\Plugins\Tasks\Discoveries; use Tempcord\Plugins\Tasks\Attributes\Task; +use Tempcord\Plugins\Tasks\Compiler\TaskCompiler; use Tempcord\Plugins\Tasks\Registry; use Tempest\Discovery\Discovery; use Tempest\Discovery\DiscoveryLocation; use Tempest\Discovery\IsDiscovery; use Tempest\Reflection\ClassReflector; -/** - * Finds every #[Task] method, wherever it lives. - * - * Discovery reaches the bot's own code and any installed package alike, so a - * package can ship scheduled tasks of its own. - */ final class TasksDiscovery implements Discovery { use IsDiscovery; public function __construct( private readonly Registry $registry, + private readonly TaskCompiler $compiler = new TaskCompiler(), ) {} public function discover(DiscoveryLocation $location, ClassReflector $class): void { - foreach ($class->getPublicMethods() as $method) { - $task = $method->getAttribute(Task::class); + foreach ($class->getAttributes(Task::class) as $task) { + $this->discoveryItems->add($location, $this->compiler->compileClass($class, $task)); + } - if ($task === null) { - continue; + foreach ($class->getPublicMethods() as $method) { + foreach ($method->getAttributes(Task::class) as $task) { + $this->discoveryItems->add($location, $this->compiler->compileMethod($class, $method, $task)); } - - $task->setReflector($method); - - $this->discoveryItems->add($location, $task); } } public function apply(): void { foreach ($this->discoveryItems as $task) { - $this->registry->register($task); + $this->registry->add($task); } } } diff --git a/src/Registry.php b/src/Registry.php index f37299c..ff8ff37 100644 --- a/src/Registry.php +++ b/src/Registry.php @@ -4,94 +4,99 @@ namespace Tempcord\Plugins\Tasks; -use Ragnarok\Fenrir\Discord; -use Ragnarok\Fenrir\Extension\Extension; -use React\EventLoop\Loop; -use Tempcord\Plugins\Tasks\Attributes\Task; +use React\EventLoop\LoopInterface; +use Tempcord\Plugins\Tasks\Definitions\TaskDefinition; use Tempcord\Plugins\Tasks\Support\TaskStats; use Tempest\Container\Container; use Tempest\Container\Singleton; use Tempest\Log\Logger; /** - * Holds every discovered task and starts them once Discord is ready. + * Holds every discovered task and puts it on the event loop. */ #[Singleton] -final class Registry implements Extension +final class Registry { - /** @var array */ + /** @var array keyed by name, so a task discovered twice is scheduled once */ private array $tasks = []; private ?Runner $runner = null; public function __construct( private readonly Container $container, - private readonly Logger $logger, ) {} - public function register(Task $task): void + public function add(TaskDefinition $task): void { - // Keyed by name so a task discovered twice is scheduled once. - $this->tasks[$task->getName()] = $task; + $this->tasks[$task->name] = $task; } /** - * Called by Fenrir once the extension is registered, which the plugin does - * as the bot boots. + * @return list */ - public function initialize(Discord $discord): void + public function all(): array + { + return array_values($this->tasks); + } + + public function count(): int + { + return count($this->tasks); + } + + /** + * Arms every task's timer. Nothing takes a turn until the loop itself runs, + * which is after the gateway opens. + * + * @return list what was scheduled, for the caller to report + */ + public function start(LoopInterface $loop): array { if ($this->tasks === []) { - return; + return []; } - $this->runner ??= new Runner(Loop::get(), $this->logger, $this->container); + /* + * Resolved here rather than in the constructor: discovery builds this + * registry while the container is still being assembled, before the + * initializers that provide the logger have themselves been found. + */ + $this->runner = new Runner($loop, $this->container, $this->container->get(Logger::class)); + + $scheduled = []; foreach ($this->tasks as $task) { - $this->runner->schedule($task); + if ($this->runner->schedule($task)) { + $scheduled[] = $task->name . ' (' . $task->schedule() . ')'; + } } - $this->logger->info('Task scheduler initialized', ['tasks' => count($this->tasks)]); + return $scheduled; } - public function cancelTask(string $taskName): bool + public function cancel(string $taskName): bool { return $this->runner?->cancel($taskName) ?? false; } - public function cancelAllTasks(): void + public function cancelAll(): void { $this->runner?->cancelAll(); } /** - * Execution statistics per task, empty until the scheduler has started. - * * @return array */ - public function getStats(): array + public function stats(): array { - return $this->runner?->getStats() ?? []; + return $this->runner?->stats() ?? []; } /** * @return list */ - public function getScheduledTasks(): array - { - return $this->runner?->getScheduledTasks() ?? []; - } - - public function count(): int - { - return count($this->tasks); - } - - /** - * @return list - */ - public function getAllTasks(): array + public function scheduled(): array { - return array_values($this->tasks); + return $this->runner?->scheduled() ?? []; } } diff --git a/src/Runner.php b/src/Runner.php index 821055a..9086b2b 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -4,150 +4,154 @@ namespace Tempcord\Plugins\Tasks; -use DateTimeImmutable; use React\EventLoop\LoopInterface; -use Tempcord\Plugins\Tasks\Attributes\Task; +use React\EventLoop\TimerInterface; +use Tempcord\Plugins\Tasks\Definitions\TaskDefinition; use Tempcord\Plugins\Tasks\Support\CronExpression; use Tempcord\Plugins\Tasks\Support\TaskStats; use Tempest\Container\Container; use Tempest\Log\Logger; use Throwable; +use function React\Async\async; + /** - * Manages the execution of scheduled tasks + * Runs scheduled tasks, and keeps them running. + * + * A timer is less forgiving than an event listener: it fires again whether or + * not the last turn finished or threw, forever. Both are contained here, so + * that a task cannot quietly stop the bot doing its other work. */ final class Runner { - /** @var array Timer references for cleanup */ + /** @var array */ private array $timers = []; - /** @var array Last run times for cron tasks */ - private array $lastRunTimes = []; + /** @var array tasks whose previous turn has not finished */ + private array $running = []; - /** @var array Task execution statistics */ + /** @var array */ private array $stats = []; public function __construct( private readonly LoopInterface $loop, - private readonly Logger $logger, private readonly Container $container, + private readonly Logger $logger, ) {} /** - * Schedule a task for execution + * @return bool whether the task was put on the schedule */ - public function schedule(Task $task): void + public function schedule(TaskDefinition $task): bool { if (!$task->enabled) { - $this->logger->info("Task '{$task->getName()}' is disabled, skipping"); - return; + $this->logger->info('Task ' . $task->name . ' is disabled and was left out of the schedule.'); + + return false; } - $taskName = $task->getName(); - $this->stats[$taskName] = new TaskStats($taskName); + $this->stats[$task->name] = new TaskStats($task->name); - if ($task->isInterval()) { - $this->scheduleIntervalTask($task); - } else { - $this->scheduleCronTask($task); + if ($task->runOnBoot) { + /* + * On the next tick rather than now, so a task cannot run before the + * rest of the bot has finished being wired together. + */ + $this->loop->futureTick(fn() => $this->run($task)); } - $this->logger->info("Scheduled task '{$taskName}'", [ - 'schedule' => $task->getScheduleDescription(), - 'runOnBoot' => $task->runOnBoot, - ]); + $task->isInterval() + ? $this->armInterval($task) + : $this->armCron($task, new CronExpression((string) $task->cron)); + + return true; } - /** - * Schedule an interval-based task - */ - private function scheduleIntervalTask(Task $task): void + private function armInterval(TaskDefinition $task): void { - $taskName = $task->getName(); - $interval = $task->interval; - - // Run immediately if configured - if ($task->runOnBoot) { - $this->loop->futureTick(fn() => $this->executeTask($task)); - } - - // Schedule periodic execution - $timer = $this->loop->addPeriodicTimer($interval, fn() => $this->executeTask($task)); - $this->timers[$taskName] = $timer; + $this->timers[$task->name] = $this->loop->addPeriodicTimer( + (int) $task->interval, + fn() => $this->run($task), + ); } /** - * Schedule a cron-based task + * Cron is armed one turn at a time, for the exact number of seconds until + * the next minute that matches. + * + * Waking every minute instead would drift: the first tick lands at whatever + * offset within the minute the bot happened to start at, and once the drift + * crosses a minute boundary a matching minute is stepped over entirely and + * the task silently does not run that hour. */ - private function scheduleCronTask(Task $task): void + private function armCron(TaskDefinition $task, CronExpression $cron): void { - $taskName = $task->getName(); - $cron = new CronExpression($task->cron); - - // Run immediately if configured - if ($task->runOnBoot) { - $this->loop->futureTick(fn() => $this->executeTask($task)); - } + $seconds = max(1, $cron->getSecondsUntilNextRun()); - // Check every minute if cron matches - $timer = $this->loop->addPeriodicTimer(60, function () use ($task, $cron, $taskName) { - $now = new DateTimeImmutable(); - $currentMinute = $now->format('Y-m-d H:i'); + $this->timers[$task->name] = $this->loop->addTimer($seconds, function () use ($task, $cron): void { + $this->run($task); - // Avoid running twice in the same minute - $lastRun = $this->lastRunTimes[$taskName] ?? null; - if ($lastRun !== null && $lastRun->format('Y-m-d H:i') === $currentMinute) { - return; - } - - if ($cron->matches($now)) { - $this->lastRunTimes[$taskName] = $now; - $this->executeTask($task); - } + // Re-armed from the turn just taken, so the wait is recomputed + // rather than accumulated. + $this->armCron($task, $cron); }); - - $this->timers[$taskName] = $timer; } - /** - * Execute a task - */ - private function executeTask(Task $task): void + private function run(TaskDefinition $task): void { - $taskName = $task->getName(); - $startTime = microtime(true); - - $this->logger->debug("Running task '{$taskName}'"); - - try { - $instance = $this->container->get($task->reflector->getDeclaringClass()->getName()); - - $task->reflector->invokeArgs($instance); + /* + * A task that takes longer than its own interval would otherwise be + * started again alongside itself, and each turn would make the next one + * slower until nothing else got a look in. + */ + if (isset($this->running[$task->name])) { + $this->logger->warning( + 'Task ' . $task->name . ' is still busy from its last turn; skipping this one.', + ); - $duration = round((microtime(true) - $startTime) * 1000, 2); - - $this->stats[$taskName]->recordSuccess($duration); + return; + } - $this->logger->info("Task '{$taskName}' completed", [ - 'duration' => "{$duration}ms", - 'runs' => $this->stats[$taskName]->totalRuns, - ]); - } catch (Throwable $e) { - $duration = round((microtime(true) - $startTime) * 1000, 2); + $this->running[$task->name] = true; + $startedAt = microtime(true); + + /* + * In a fiber, so a task may await the REST API the way a command + * handler does, and inside a catch, so one that throws is logged rather + * than travelling up into the event loop and taking the process with it. + */ + async(function () use ($task, $startedAt): void { + try { + $task->method->invokeArgs($this->container->get($task->handler), []); + + $this->record($task)?->recordSuccess($this->msSince($startedAt)); + + // Debug, not info: a task running every ten seconds would + // otherwise write eight thousand lines a day saying nothing. + $this->logger->debug('Task ' . $task->name . ' finished.'); + } catch (Throwable $throwable) { + $this->record($task)?->recordFailure($this->msSince($startedAt), $throwable->getMessage()); + + $this->logger->error( + 'Task ' . $task->name . ' failed: ' . $throwable->getMessage(), + ['exception' => $throwable], + ); + } finally { + unset($this->running[$task->name]); + } + })(); + } - $this->stats[$taskName]->recordFailure($duration, $e->getMessage()); + private function record(TaskDefinition $task): ?TaskStats + { + return $this->stats[$task->name] ?? null; + } - $this->logger->error("Task '{$taskName}' failed", [ - 'error' => $e->getMessage(), - 'duration' => "{$duration}ms", - 'failures' => $this->stats[$taskName]->failures, - ]); - } + private function msSince(float $startedAt): float + { + return round((microtime(true) - $startedAt) * 1000, 2); } - /** - * Cancel a scheduled task - */ public function cancel(string $taskName): bool { if (!isset($this->timers[$taskName])) { @@ -157,14 +161,11 @@ public function cancel(string $taskName): bool $this->loop->cancelTimer($this->timers[$taskName]); unset($this->timers[$taskName]); - $this->logger->info("Cancelled task '{$taskName}'"); + $this->logger->info('Task ' . $taskName . ' was cancelled.'); return true; } - /** - * Cancel all scheduled tasks - */ public function cancelAll(): void { foreach (array_keys($this->timers) as $taskName) { @@ -172,10 +173,7 @@ public function cancelAll(): void } } - /** - * Get statistics for a specific task - */ - public function getTaskStats(string $taskName): ?TaskStats + public function statsFor(string $taskName): ?TaskStats { return $this->stats[$taskName] ?? null; } @@ -183,16 +181,15 @@ public function getTaskStats(string $taskName): ?TaskStats /** * @return array */ - public function getStats(): array + public function stats(): array { return $this->stats; } /** - * Get list of scheduled task names - * @return array + * @return list */ - public function getScheduledTasks(): array + public function scheduled(): array { return array_keys($this->timers); } diff --git a/src/Support/CronExpression.php b/src/Support/CronExpression.php index 005d4d5..84b9950 100644 --- a/src/Support/CronExpression.php +++ b/src/Support/CronExpression.php @@ -7,93 +7,117 @@ use DateTimeImmutable; use DateTimeInterface; use InvalidArgumentException; +use RuntimeException; /** - * Simple cron expression parser + * A five field cron expression. * - * Supports standard 5-field cron format: - * ┌───────────── minute (0-59) - * │ ┌───────────── hour (0-23) - * │ │ ┌───────────── day of month (1-31) - * │ │ │ ┌───────────── month (1-12) - * │ │ │ │ ┌───────────── day of week (0-6, Sunday = 0) - * │ │ │ │ │ - * * * * * * + * ┌───────────── minute (0-59) + * │ ┌───────────── hour (0-23) + * │ │ ┌───────────── day of month (1-31) + * │ │ │ ┌───────────── month (1-12) + * │ │ │ │ ┌───────────── day of week (0-6, Sunday = 0) + * │ │ │ │ │ + * * * * * * */ final class CronExpression { + /** @var list */ private array $minutes; + + /** @var list */ private array $hours; + + /** @var list */ private array $daysOfMonth; + + /** @var list */ private array $months; + + /** @var list */ private array $daysOfWeek; + /** + * Whether each of the two day fields names particular days rather than + * standing open. Cron reads the pair as "or" when both are restricted. + */ + private bool $dayOfMonthRestricted; + + private bool $dayOfWeekRestricted; + public function __construct( - public readonly string $expression + public readonly string $expression, ) { $this->parse($expression); } + public function matches(DateTimeInterface $moment): bool + { + $matchesTime = in_array((int) $moment->format('i'), $this->minutes, true) + && in_array((int) $moment->format('G'), $this->hours, true) + && in_array((int) $moment->format('n'), $this->months, true); + + return $matchesTime && $this->matchesDay($moment); + } + /** - * Check if the cron expression matches the given time + * Cron's one genuine oddity: when a line restricts the day of the month + * *and* the day of the week, it runs on days matching either, not both. So + * `0 0 1 * 1` is the first of the month and every Monday, which is what + * anyone writing it expects and not what an "and" would give them. */ - public function matches(DateTimeInterface $dateTime): bool + private function matchesDay(DateTimeInterface $moment): bool { - $minute = (int) $dateTime->format('i'); - $hour = (int) $dateTime->format('G'); - $dayOfMonth = (int) $dateTime->format('j'); - $month = (int) $dateTime->format('n'); - $dayOfWeek = (int) $dateTime->format('w'); - - return in_array($minute, $this->minutes, true) - && in_array($hour, $this->hours, true) - && in_array($dayOfMonth, $this->daysOfMonth, true) - && in_array($month, $this->months, true) - && in_array($dayOfWeek, $this->daysOfWeek, true); + $dayOfMonth = in_array((int) $moment->format('j'), $this->daysOfMonth, true); + $dayOfWeek = in_array((int) $moment->format('w'), $this->daysOfWeek, true); + + if ($this->dayOfMonthRestricted && $this->dayOfWeekRestricted) { + return $dayOfMonth || $dayOfWeek; + } + + return $dayOfMonth && $dayOfWeek; } /** - * Get the next run time after the given time + * The first minute at or after the one following the given moment that this + * expression matches. */ public function getNextRunDate(DateTimeInterface $from): DateTimeImmutable { - $next = DateTimeImmutable::createFromInterface($from); - $next = $next->modify('+1 minute')->setTime( - (int) $next->modify('+1 minute')->format('G'), - (int) $next->modify('+1 minute')->format('i'), - 0 - ); + $next = DateTimeImmutable::createFromInterface($from) + ->modify('+1 minute') + ->setTime( + (int) DateTimeImmutable::createFromInterface($from)->modify('+1 minute')->format('G'), + (int) DateTimeImmutable::createFromInterface($from)->modify('+1 minute')->format('i'), + 0, + ); - // Search for up to 4 years to find next match - $maxIterations = 60 * 24 * 366 * 4; + // A day of the month that never comes round in a given month — the 31st + // of February — still resolves within four years, or not at all. + $limit = 60 * 24 * 366 * 4; - for ($i = 0; $i < $maxIterations; $i++) { + for ($minute = 0; $minute < $limit; $minute++) { if ($this->matches($next)) { return $next; } + $next = $next->modify('+1 minute'); } - throw new \RuntimeException('Could not find next run date within 4 years'); + throw new RuntimeException( + 'The expression "' . $this->expression . '" does not come round within four years.', + ); } - /** - * Get seconds until the next run - */ public function getSecondsUntilNextRun(?DateTimeInterface $from = null): int { $from ??= new DateTimeImmutable(); - $next = $this->getNextRunDate($from); - return $next->getTimestamp() - $from->getTimestamp(); + return $this->getNextRunDate($from)->getTimestamp() - $from->getTimestamp(); } - /** - * Parse the cron expression - */ private function parse(string $expression): void { - // Handle common aliases $expression = match (strtolower(trim($expression))) { '@yearly', '@annually' => '0 0 1 1 *', '@monthly' => '0 0 1 * *', @@ -103,11 +127,11 @@ private function parse(string $expression): void default => trim($expression), }; - $parts = preg_split('/\s+/', $expression); + $parts = preg_split('/\s+/', $expression) ?: []; if (count($parts) !== 5) { throw new InvalidArgumentException( - "Invalid cron expression '{$expression}'. Expected 5 fields: minute hour day month weekday" + 'Invalid cron expression "' . $expression . '". Expected 5 fields: minute hour day month weekday', ); } @@ -118,68 +142,71 @@ private function parse(string $expression): void $this->daysOfMonth = $this->parseField($dayOfMonth, 1, 31); $this->months = $this->parseField($month, 1, 12); $this->daysOfWeek = $this->parseField($dayOfWeek, 0, 6); + + $this->dayOfMonthRestricted = trim($dayOfMonth) !== '*'; + $this->dayOfWeekRestricted = trim($dayOfWeek) !== '*'; } /** - * Parse a single cron field + * @return list */ private function parseField(string $field, int $min, int $max): array { $values = []; - // Handle comma-separated values - $parts = explode(',', $field); - - foreach ($parts as $part) { - $values = array_merge($values, $this->parsePart($part, $min, $max)); + foreach (explode(',', $field) as $part) { + $values = [...$values, ...$this->parsePart($part, $min, $max)]; } - $values = array_unique($values); + $values = array_values(array_unique($values)); sort($values); return $values; } /** - * Parse a single part of a cron field + * @return list */ private function parsePart(string $part, int $min, int $max): array { - // Handle wildcard (*) if ($part === '*') { return range($min, $max); } - // Handle step values (*/5, 1-10/2) if (str_contains($part, '/')) { [$range, $step] = explode('/', $part, 2); $step = (int) $step; - if ($range === '*') { - $rangeValues = range($min, $max); - } else { - $rangeValues = $this->parsePart($range, $min, $max); + if ($step < 1) { + throw new InvalidArgumentException('A cron step must be at least 1, got "' . $part . '".'); } + $values = $range === '*' ? range($min, $max) : $this->parsePart($range, $min, $max); + + /* + * Counted from where the range starts, not from the bottom of the + * field: 1-10/2 is 1,3,5,7,9 — every second value beginning at one + * — and not 2,4,6,8,10. + */ + $start = $values[0]; + return array_values(array_filter( - $rangeValues, - fn($v) => ($v - $min) % $step === 0 + $values, + static fn(int $value) => ($value - $start) % $step === 0, )); } - // Handle ranges (1-5) if (str_contains($part, '-')) { - [$start, $end] = explode('-', $part, 2); - $start = max($min, (int) $start); - $end = min($max, (int) $end); - return range($start, $end); + [$from, $to] = explode('-', $part, 2); + + return range(max($min, (int) $from), min($max, (int) $to)); } - // Handle single value $value = (int) $part; - if ($value < $min || $value > $max) { + + if ($value < $min || $value > $max || !ctype_digit(trim($part))) { throw new InvalidArgumentException( - "Value {$value} is out of range [{$min}-{$max}]" + 'Value "' . $part . '" is out of range [' . $min . '-' . $max . ']', ); } diff --git a/src/TasksPlugin.php b/src/TasksPlugin.php index 934b453..4a20472 100644 --- a/src/TasksPlugin.php +++ b/src/TasksPlugin.php @@ -4,24 +4,35 @@ namespace Tempcord\Plugins\Tasks; +use React\EventLoop\Loop; use Tempcord\Plugins\Plugin; use Tempcord\Tempcord; +use Tempest\Log\Logger; /** - * Interval and cron scheduling for Tempcord bots. + * Puts every discovered task on the event loop. * - * The #[Task] attribute is found by TasksDiscovery wherever it appears, in the - * bot's own code or in another package. This only has to hand the registry to - * Discord, so its timers start with the gateway. + * A plugin boots after commands and events are bound and before the gateway + * opens, which is exactly when timers want arming: nothing fires until the loop + * itself runs, so no task can take a turn against a half-built bot. */ final readonly class TasksPlugin implements Plugin { public function __construct( private Registry $registry, + private Logger $logger, ) {} public function boot(Tempcord $tempcord): void { - $tempcord->discord->registerExtension($this->registry); + $scheduled = $this->registry->start(Loop::get()); + + if ($scheduled === []) { + return; + } + + $this->logger->info( + 'Scheduled ' . count($scheduled) . ' task(s): ' . implode(', ', $scheduled), + ); } } diff --git a/src/functions.php b/src/functions.php index a61a09c..c8d12ba 100644 --- a/src/functions.php +++ b/src/functions.php @@ -5,14 +5,13 @@ namespace Tempcord\Plugins\Tasks; use Tempcord\Plugins\Tasks\Support\TaskStats; + use function Tempest\Container\get; if (!function_exists('Tempcord\Plugins\Tasks\tasks')) { /** - * The task registry, for inspecting or cancelling scheduled tasks. - * - * Prefer injecting Registry where you can; this exists for the places a - * container is awkward to reach, such as a closure in configuration. + * The task registry, for reaching the schedule from somewhere the container + * does not inject into. */ function tasks(): Registry { @@ -21,7 +20,7 @@ function tasks(): Registry function cancelTask(string $taskName): bool { - return tasks()->cancelTask($taskName); + return tasks()->cancel($taskName); } /** @@ -29,6 +28,6 @@ function cancelTask(string $taskName): bool */ function taskStats(): array { - return tasks()->getStats(); + return tasks()->stats(); } } diff --git a/tests/Doubles/FakeDiscord.php b/tests/Doubles/FakeDiscord.php deleted file mode 100644 index 9a968d3..0000000 --- a/tests/Doubles/FakeDiscord.php +++ /dev/null @@ -1,33 +0,0 @@ -newInstanceWithoutConstructor(); - $gateway->events = new EventHandler($mapper); - - $this->gateway = $gateway; - } -} diff --git a/tests/Doubles/FakeLoop.php b/tests/Doubles/FakeLoop.php new file mode 100644 index 0000000..7e40a5e --- /dev/null +++ b/tests/Doubles/FakeLoop.php @@ -0,0 +1,102 @@ + */ + public array $timers = []; + + /** @var list */ + public array $futureTicks = []; + + public function addTimer($interval, $callback): TimerInterface + { + $timer = new FakeTimer((float) $interval, $callback, periodic: false); + $this->timers[] = $timer; + + return $timer; + } + + public function addPeriodicTimer($interval, $callback): TimerInterface + { + $timer = new FakeTimer((float) $interval, $callback, periodic: true); + $this->timers[] = $timer; + + return $timer; + } + + public function cancelTimer(TimerInterface $timer): void + { + $this->timers = array_values(array_filter( + $this->timers, + static fn(FakeTimer $known) => $known !== $timer, + )); + } + + public function futureTick($listener): void + { + $this->futureTicks[] = $listener; + } + + /** + * Fires every timer currently armed, once. + * + * The list is copied first: a cron task re-arms itself from inside its own + * callback, and the new timer belongs to the next turn rather than this one. + */ + public function tick(int $times = 1): void + { + for ($turn = 0; $turn < $times; $turn++) { + foreach ($this->timers as $timer) { + $timer->fire(); + } + } + } + + public function drainFutureTicks(): void + { + $ticks = $this->futureTicks; + $this->futureTicks = []; + + foreach ($ticks as $tick) { + $tick(); + } + } + + /** + * The most recently armed timer, which for a cron task is the one waiting + * for the next matching minute. + */ + public function lastTimer(): ?FakeTimer + { + return $this->timers === [] ? null : $this->timers[count($this->timers) - 1]; + } + + public function addReadStream($stream, $listener): void {} + + public function addWriteStream($stream, $listener): void {} + + public function removeReadStream($stream): void {} + + public function removeWriteStream($stream): void {} + + public function addSignal($signal, $listener): void {} + + public function removeSignal($signal, $listener): void {} + + public function run(): void {} + + public function stop(): void {} +} diff --git a/tests/Doubles/FakeTimer.php b/tests/Doubles/FakeTimer.php new file mode 100644 index 0000000..66983d7 --- /dev/null +++ b/tests/Doubles/FakeTimer.php @@ -0,0 +1,36 @@ +interval; + } + + public function getCallback(): callable + { + return $this->callback; + } + + public function isPeriodic(): bool + { + return $this->periodic; + } + + public function fire(): void + { + ($this->callback)($this); + } +} diff --git a/tests/Doubles/RecordingLogger.php b/tests/Doubles/RecordingLogger.php new file mode 100644 index 0000000..d525618 --- /dev/null +++ b/tests/Doubles/RecordingLogger.php @@ -0,0 +1,35 @@ + */ + public array $messages = []; + + /** @var list the level each message was logged at, in step with $messages */ + public array $levels = []; + + public function log($level, string|Stringable $message, array $context = []): void + { + $this->messages[] = (string) $message; + $this->levels[] = (string) $level; + } + + public function has(string $needle): bool + { + foreach ($this->messages as $message) { + if (str_contains($message, $needle)) { + return true; + } + } + + return false; + } +} diff --git a/tests/Fixtures/BootTask.php b/tests/Fixtures/BootTask.php new file mode 100644 index 0000000..d5a2c5d --- /dev/null +++ b/tests/Fixtures/BootTask.php @@ -0,0 +1,22 @@ +promise()); + } +} diff --git a/tests/Fixtures/SweepMessages.php b/tests/Fixtures/SweepMessages.php new file mode 100644 index 0000000..7d4f5ce --- /dev/null +++ b/tests/Fixtures/SweepMessages.php @@ -0,0 +1,21 @@ +createStub(Logger::class)); - } - - private function task(string $method, ?string $name = null): Task - { - $task = new Task(interval: 60, name: $name); - $task->setReflector(new ClassReflector(ScheduledCommands::class)->getMethod($method)); - - return $task; - } - - public function test_it_holds_registered_tasks(): void - { - $registry = $this->registry(); - $registry->register($this->task('everyMinute')); - - $this->assertSame(1, $registry->count()); - $this->assertSame('everyMinute', $registry->getAllTasks()[0]->getName()); - } - - /** - * Discovery can reach the same method through more than one location, and - * scheduling it twice would run it twice on every tick. - */ - public function test_the_same_task_registered_twice_is_held_once(): void - { - $registry = $this->registry(); - $registry->register($this->task('everyMinute')); - $registry->register($this->task('everyMinute')); - - $this->assertSame(1, $registry->count()); - } - - /** - * Statistics come from the runner, which does not exist until the scheduler - * starts. This used to return the runner itself, so callers expecting a map - * of statistics got an object. - */ - public function test_statistics_are_empty_before_the_scheduler_starts(): void - { - $registry = $this->registry(); - $registry->register($this->task('everyMinute')); - - $this->assertSame([], $registry->getStats()); - $this->assertSame([], $registry->getScheduledTasks()); - } - - public function test_cancelling_before_the_scheduler_starts_reports_failure(): void - { - $this->assertFalse($this->registry()->cancelTask('everyMinute')); - } -} diff --git a/tests/RunnerTest.php b/tests/RunnerTest.php deleted file mode 100644 index 8c74e7a..0000000 --- a/tests/RunnerTest.php +++ /dev/null @@ -1,113 +0,0 @@ -createStub(Logger::class), new GenericContainer()); - } - - private function task(string $method, ?int $interval = 60, ?string $cron = null, bool $enabled = true, bool $runOnBoot = false): Task - { - $task = new Task(interval: $interval, cron: $cron, runOnBoot: $runOnBoot, enabled: $enabled); - $task->setReflector(new ClassReflector(ScheduledCommands::class)->getMethod($method)); - - return $task; - } - - public function test_it_schedules_an_interval_task(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('everyMinute')); - - $this->assertSame(['everyMinute'], $runner->getScheduledTasks()); - - $runner->cancelAll(); - } - - public function test_a_disabled_task_is_never_scheduled(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('disabled', enabled: false)); - - $this->assertSame([], $runner->getScheduledTasks()); - $this->assertNull($runner->getTaskStats('disabled')); - } - - public function test_a_cron_task_is_scheduled_on_a_ticking_timer(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('report', interval: null, cron: '0 * * * *')); - - $this->assertSame(['report'], $runner->getScheduledTasks()); - - $runner->cancelAll(); - } - - public function test_cancelling_removes_a_task(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('everyMinute')); - - $this->assertTrue($runner->cancel('everyMinute')); - $this->assertSame([], $runner->getScheduledTasks()); - $this->assertFalse($runner->cancel('everyMinute')); - } - - /** - * Statistics exist from the moment a task is scheduled, so a task that has - * not run yet still reports zero runs rather than nothing at all. - */ - public function test_statistics_exist_from_scheduling(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('everyMinute')); - - $stats = $runner->getStats(); - - $this->assertArrayHasKey('everyMinute', $stats); - $this->assertSame(0, $stats['everyMinute']->totalRuns); - - $runner->cancelAll(); - } - - /** - * runOnBoot queues the task on the next tick rather than running it inline, - * so scheduling never blocks the boot sequence. - */ - public function test_run_on_boot_executes_the_task(): void - { - $runner = $this->runner(); - $runner->schedule($this->task('everyMinute', runOnBoot: true)); - - $this->assertSame([], ScheduledCommands::$ran, 'should not have run inline'); - - Loop::get()->futureTick(static fn() => Loop::get()->stop()); - Loop::get()->run(); - - $this->assertSame(['everyMinute'], ScheduledCommands::$ran); - $this->assertSame(1, $runner->getStats()['everyMinute']->totalRuns); - - $runner->cancelAll(); - } -} diff --git a/tests/TaskTest.php b/tests/TaskTest.php deleted file mode 100644 index b5ab3dd..0000000 --- a/tests/TaskTest.php +++ /dev/null @@ -1,99 +0,0 @@ -expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('either an interval or cron expression'); - - new Task(); - } - - public function test_a_task_cannot_have_both_kinds_of_schedule(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('cannot have both'); - - new Task(interval: 60, cron: '* * * * *'); - } - - public function test_an_interval_below_a_second_is_rejected(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('at least 1 second'); - - new Task(interval: 0); - } - - /** - * The name falls back to the method it sits on, which is only known once - * discovery has attached the reflector. - */ - public function test_the_name_falls_back_to_the_method(): void - { - $task = new Task(interval: 60); - - $this->assertSame('unknown', $task->getName()); - - $task->setReflector(new ClassReflector(ScheduledCommands::class)->getMethod('everyMinute')); - - $this->assertSame('everyMinute', $task->getName()); - } - - public function test_an_explicit_name_wins(): void - { - $task = new Task(cron: '0 * * * *', name: 'hourly-report'); - $task->setReflector(new ClassReflector(ScheduledCommands::class)->getMethod('report')); - - $this->assertSame('hourly-report', $task->getName()); - } - - /** @return array */ - public static function intervals(): array - { - return [ - 'one second' => [1, 'every 1 second'], - 'seconds' => [30, 'every 30 seconds'], - 'one minute' => [60, 'every 1 minute'], - 'minutes' => [300, 'every 5 minutes'], - 'hours' => [7200, 'every 2 hours'], - 'days' => [172800, 'every 2 days'], - ]; - } - - #[DataProvider('intervals')] - public function test_it_describes_an_interval_in_words(int $seconds, string $expected): void - { - $this->assertSame($expected, new Task(interval: $seconds)->getScheduleDescription()); - } - - public function test_it_describes_a_cron_schedule(): void - { - $this->assertSame('cron: 0 * * * *', new Task(cron: '0 * * * *')->getScheduleDescription()); - } - - public function test_it_knows_which_kind_of_schedule_it_has(): void - { - $interval = new Task(interval: 60); - $cron = new Task(cron: '* * * * *'); - - $this->assertTrue($interval->isInterval()); - $this->assertFalse($interval->isCron()); - $this->assertTrue($cron->isCron()); - $this->assertFalse($cron->isInterval()); - } -} diff --git a/tests/TasksDiscoveryTest.php b/tests/TasksDiscoveryTest.php deleted file mode 100644 index 89db097..0000000 --- a/tests/TasksDiscoveryTest.php +++ /dev/null @@ -1,107 +0,0 @@ -location = new DiscoveryLocation( - namespace: 'Tempcord\\Plugins\\Tasks\\Tests\\Fixtures\\', - path: __DIR__ . '/Fixtures', - ); - } - - private function discovery(Registry $registry): TasksDiscovery - { - $discovery = new TasksDiscovery($registry); - $discovery->setItems(new DiscoveryItems()); - - return $discovery; - } - - private function registry(): Registry - { - return new Registry(new GenericContainer(), $this->createStub(Logger::class)); - } - - public function test_it_finds_every_annotated_method(): void - { - $registry = $this->registry(); - $discovery = $this->discovery($registry); - - $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); - $discovery->apply(); - - $names = array_map(static fn($task) => $task->getName(), $registry->getAllTasks()); - - sort($names); - - $this->assertSame(['disabled', 'everyMinute', 'hourly-report'], $names); - } - - /** - * A method without the attribute is left alone, so helpers can sit beside - * scheduled work. - */ - public function test_it_ignores_methods_without_the_attribute(): void - { - $registry = $this->registry(); - $discovery = $this->discovery($registry); - - $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); - $discovery->apply(); - - $names = array_map(static fn($task) => $task->getName(), $registry->getAllTasks()); - - $this->assertNotContains('notATask', $names); - } - - /** - * The reflector is what lets the runner find and call the method later, so - * discovery has to attach it. - */ - public function test_it_attaches_the_method_to_each_task(): void - { - $registry = $this->registry(); - $discovery = $this->discovery($registry); - - $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); - $discovery->apply(); - - foreach ($registry->getAllTasks() as $task) { - $this->assertNotNull($task->reflector); - $this->assertSame(ScheduledCommands::class, $task->reflector->getDeclaringClass()->getName()); - } - } - - /** - * Registration happens in apply, not while discovering, so a discovery run - * that is thrown away leaves no trace. - */ - public function test_nothing_is_registered_until_apply(): void - { - $registry = $this->registry(); - $discovery = $this->discovery($registry); - - $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); - - $this->assertSame(0, $registry->count()); - } -} diff --git a/tests/TasksPluginTest.php b/tests/TasksPluginTest.php deleted file mode 100644 index 9bcc007..0000000 --- a/tests/TasksPluginTest.php +++ /dev/null @@ -1,56 +0,0 @@ -assertInstanceOf( - Plugin::class, - new TasksPlugin(new Registry(new GenericContainer(), $this->createStub(Logger::class))), - ); - } - - /** - * The registry is a Fenrir extension: registering it is what gets its - * timers started once the gateway is up. - */ - public function test_booting_registers_the_registry_as_a_discord_extension(): void - { - $registry = new Registry(new GenericContainer(), $this->createStub(Logger::class)); - $discord = new FakeDiscord(); - - new TasksPlugin($registry)->boot($this->tempcord($discord)); - - $this->assertTrue($discord->hasExtension(Registry::class)); - $this->assertSame($registry, $discord->getExtension(Registry::class)); - } - - private function tempcord(Discord $discord): Tempcord - { - /* - * Only the discord property is touched during boot, so the rest is - * built without a container. - */ - $tempcord = new \ReflectionClass(Tempcord::class)->newInstanceWithoutConstructor(); - - new \ReflectionProperty(Tempcord::class, 'discord')->setValue($tempcord, $discord); - - return $tempcord; - } -} diff --git a/tests/Unit/CronExpressionTest.php b/tests/Unit/CronExpressionTest.php new file mode 100644 index 0000000..53bb861 --- /dev/null +++ b/tests/Unit/CronExpressionTest.php @@ -0,0 +1,195 @@ +assertTrue($cron->matches($this->at('2026-09-02 13:37:00'))); + $this->assertTrue($cron->matches($this->at('2026-01-01 00:00:00'))); + } + + public function test_a_fixed_minute_and_hour_matches_only_then(): void + { + $cron = new CronExpression('30 14 * * *'); + + $this->assertTrue($cron->matches($this->at('2026-09-02 14:30:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 14:31:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 15:30:00'))); + } + + #[DataProvider('aliases')] + public function test_an_alias_stands_for_its_expression(string $alias, string $matches, string $misses): void + { + $cron = new CronExpression($alias); + + $this->assertTrue($cron->matches($this->at($matches)), $alias . ' should match ' . $matches); + $this->assertFalse($cron->matches($this->at($misses)), $alias . ' should not match ' . $misses); + } + + /** + * @return array + */ + public static function aliases(): array + { + return [ + '@hourly' => ['@hourly', '2026-09-02 14:00:00', '2026-09-02 14:01:00'], + '@daily' => ['@daily', '2026-09-02 00:00:00', '2026-09-02 01:00:00'], + '@midnight' => ['@midnight', '2026-09-02 00:00:00', '2026-09-02 12:00:00'], + '@weekly' => ['@weekly', '2026-09-06 00:00:00', '2026-09-07 00:00:00'], + '@monthly' => ['@monthly', '2026-09-01 00:00:00', '2026-09-02 00:00:00'], + '@yearly' => ['@yearly', '2026-01-01 00:00:00', '2026-02-01 00:00:00'], + ]; + } + + public function test_a_list_matches_any_of_its_values(): void + { + $cron = new CronExpression('0,15,30,45 * * * *'); + + $this->assertTrue($cron->matches($this->at('2026-09-02 14:15:00'))); + $this->assertTrue($cron->matches($this->at('2026-09-02 14:45:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 14:20:00'))); + } + + public function test_a_range_matches_within_it(): void + { + $cron = new CronExpression('0 9-17 * * *'); + + $this->assertTrue($cron->matches($this->at('2026-09-02 09:00:00'))); + $this->assertTrue($cron->matches($this->at('2026-09-02 17:00:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 18:00:00'))); + } + + public function test_a_step_over_a_wildcard_counts_from_the_bottom_of_the_field(): void + { + $cron = new CronExpression('*/15 * * * *'); + + $this->assertTrue($cron->matches($this->at('2026-09-02 14:00:00'))); + $this->assertTrue($cron->matches($this->at('2026-09-02 14:30:00'))); + $this->assertFalse($cron->matches($this->at('2026-09-02 14:10:00'))); + } + + /** + * A step over a range counts from where the range starts: 1-10/2 is every + * second value beginning at one, so 1,3,5,7,9 — not 2,4,6,8,10. + */ + public function test_a_step_over_a_range_counts_from_the_start_of_the_range(): void + { + $cron = new CronExpression('1-10/2 * * * *'); + + foreach ([1, 3, 5, 7, 9] as $minute) { + $this->assertTrue($cron->matches($this->at(sprintf('2026-09-02 14:%02d:00', $minute)))); + } + + foreach ([2, 4, 10] as $minute) { + $this->assertFalse($cron->matches($this->at(sprintf('2026-09-02 14:%02d:00', $minute)))); + } + } + + /** + * Cron's one genuine oddity: with both day fields restricted the line runs + * on days matching either. + */ + public function test_restricting_both_day_fields_matches_either_of_them(): void + { + $cron = new CronExpression('0 0 1 * 1'); + + // The first of the month, which in July 2026 is a Wednesday. + $this->assertTrue($cron->matches($this->at('2026-07-01 00:00:00'))); + // An ordinary Monday, which is not the first. + $this->assertTrue($cron->matches($this->at('2026-07-06 00:00:00'))); + // Neither. + $this->assertFalse($cron->matches($this->at('2026-07-07 00:00:00'))); + } + + /** + * With only one of them restricted the other stands open and the usual + * reading applies. + */ + public function test_restricting_one_day_field_still_narrows(): void + { + $cron = new CronExpression('0 0 1 * *'); + + $this->assertTrue($cron->matches($this->at('2026-07-01 00:00:00'))); + $this->assertFalse($cron->matches($this->at('2026-07-06 00:00:00'))); + } + + public function test_the_next_run_is_the_following_matching_minute(): void + { + $next = new CronExpression('0 * * * *')->getNextRunDate($this->at('2026-09-02 14:30:12')); + + $this->assertSame('2026-09-02 15:00:00', $next->format('Y-m-d H:i:s')); + } + + /** + * The minute the expression is asked from is behind it; asking at exactly + * the matching minute gives the next one rather than answering with now. + */ + public function test_the_next_run_is_never_the_minute_it_was_asked_in(): void + { + $next = new CronExpression('* * * * *')->getNextRunDate($this->at('2026-09-02 14:30:00')); + + $this->assertSame('2026-09-02 14:31:00', $next->format('Y-m-d H:i:s')); + } + + public function test_it_says_how_long_until_the_next_run(): void + { + $seconds = new CronExpression('0 * * * *') + ->getSecondsUntilNextRun($this->at('2026-09-02 14:59:30')); + + $this->assertSame(30, $seconds); + } + + public function test_an_expression_with_the_wrong_number_of_fields_is_refused(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected 5 fields'); + + new CronExpression('0 0 *'); + } + + public function test_a_value_outside_its_field_is_refused(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('out of range'); + + new CronExpression('99 * * * *'); + } + + /** + * Anything that is not a number would otherwise be cast to zero, and a task + * written for noon would quietly run at midnight. + */ + public function test_something_that_is_not_a_number_is_refused(): void + { + $this->expectException(InvalidArgumentException::class); + + new CronExpression('noon * * * *'); + } + + public function test_a_step_of_zero_is_refused(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('step must be at least 1'); + + new CronExpression('*/0 * * * *'); + } +} diff --git a/tests/Unit/CronSchedulingTest.php b/tests/Unit/CronSchedulingTest.php new file mode 100644 index 0000000..1820297 --- /dev/null +++ b/tests/Unit/CronSchedulingTest.php @@ -0,0 +1,106 @@ +loop = new FakeLoop(); + $this->runner = new Runner($this->loop, new GenericContainer(), new RecordingLogger()); + + $reflector = new ClassReflector(MinutelyTask::class); + + /** @var Task $attribute */ + $attribute = $reflector->getAttribute(Task::class); + + $this->runner->schedule(new TaskCompiler()->compileClass($reflector, $attribute)); + } + + /** + * A periodic timer is exactly what drifts, so a cron task must not be given + * one. + */ + public function test_a_cron_task_is_armed_one_turn_at_a_time(): void + { + $this->assertCount(1, $this->loop->timers); + $this->assertFalse($this->loop->timers[0]->isPeriodic()); + } + + public function test_it_waits_only_until_the_next_matching_minute(): void + { + $wait = $this->loop->timers[0]->getInterval(); + + $this->assertGreaterThan(0, $wait); + $this->assertLessThanOrEqual(60, $wait); + } + + public function test_the_turn_runs_when_its_moment_arrives(): void + { + $this->loop->timers[0]->fire(); + + $this->assertSame(1, MinutelyTask::$turns); + } + + /** + * Re-armed from the turn just taken, so the wait is recomputed rather than + * accumulated. + */ + public function test_taking_a_turn_arms_the_next_one(): void + { + $first = $this->loop->timers[0]; + $first->fire(); + + $armed = $this->loop->lastTimer(); + + $this->assertNotNull($armed); + $this->assertNotSame($first, $armed); + $this->assertFalse($armed->isPeriodic()); + $this->assertGreaterThan(0, $armed->getInterval()); + $this->assertLessThanOrEqual(60, $armed->getInterval()); + } + + public function test_it_keeps_taking_turns(): void + { + for ($turn = 0; $turn < 3; $turn++) { + $this->loop->lastTimer()?->fire(); + } + + $this->assertSame(3, MinutelyTask::$turns); + } + + public function test_a_cron_task_can_be_cancelled(): void + { + $this->assertTrue($this->runner->cancel('MinutelyTask')); + $this->assertSame([], $this->runner->scheduled()); + } +} diff --git a/tests/Unit/RegistryTest.php b/tests/Unit/RegistryTest.php new file mode 100644 index 0000000..de0a41d --- /dev/null +++ b/tests/Unit/RegistryTest.php @@ -0,0 +1,119 @@ +container = new GenericContainer(); + $this->container->singleton(Logger::class, new RecordingLogger()); + } + + private function registry(string ...$classes): Registry + { + $registry = new Registry($this->container); + + foreach ($classes as $class) { + $registry->add($this->definition($class)); + } + + return $registry; + } + + private function definition(string $class): TaskDefinition + { + $reflector = new ClassReflector($class); + + /** @var Task $attribute */ + $attribute = $reflector->getAttribute(Task::class); + + return new TaskCompiler()->compileClass($reflector, $attribute); + } + + public function test_it_holds_registered_tasks(): void + { + $registry = $this->registry(SweepMessages::class, DisabledTask::class); + + $this->assertSame(2, $registry->count()); + $this->assertSame( + ['SweepMessages', 'DisabledTask'], + array_map(static fn($task) => $task->name, $registry->all()), + ); + } + + /** + * Discovery can reach the same class twice — from the bot's own code and + * from a package that ships it — and the task must still run once. + */ + public function test_the_same_task_registered_twice_is_held_once(): void + { + $registry = $this->registry(SweepMessages::class, SweepMessages::class); + + $this->assertSame(1, $registry->count()); + } + + public function test_starting_puts_every_enabled_task_on_the_loop(): void + { + $loop = new FakeLoop(); + $scheduled = $this->registry(SweepMessages::class, DisabledTask::class)->start($loop); + + $this->assertCount(1, $scheduled); + $this->assertStringContainsString('SweepMessages', $scheduled[0]); + $this->assertStringContainsString('every 10 seconds', $scheduled[0]); + $this->assertCount(1, $loop->timers); + } + + /** + * A bot with nothing scheduled must not build a runner, and must not report + * having started one. + */ + public function test_a_bot_with_no_tasks_starts_nothing(): void + { + $this->assertSame([], $this->registry()->start(new FakeLoop())); + } + + public function test_statistics_are_empty_before_the_scheduler_starts(): void + { + $this->assertSame([], $this->registry(SweepMessages::class)->stats()); + } + + public function test_cancelling_before_the_scheduler_starts_reports_failure(): void + { + $this->assertFalse($this->registry(SweepMessages::class)->cancel('SweepMessages')); + } + + public function test_a_started_task_can_be_cancelled_through_the_registry(): void + { + $registry = $this->registry(SweepMessages::class); + $loop = new FakeLoop(); + $registry->start($loop); + + $this->assertTrue($registry->cancel('SweepMessages')); + + $loop->tick(3); + + $this->assertSame(0, SweepMessages::$turns); + } +} diff --git a/tests/Unit/RunnerTest.php b/tests/Unit/RunnerTest.php new file mode 100644 index 0000000..1ad29df --- /dev/null +++ b/tests/Unit/RunnerTest.php @@ -0,0 +1,268 @@ +loop = new FakeLoop(); + $this->logger = new RecordingLogger(); + $this->container = new GenericContainer(); + } + + private function definition(string $class): TaskDefinition + { + $reflector = new ClassReflector($class); + + /** @var Task $attribute */ + $attribute = $reflector->getAttribute(Task::class); + + return new TaskCompiler()->compileClass($reflector, $attribute); + } + + private function runner(): Runner + { + return new Runner($this->loop, $this->container, $this->logger); + } + + private function schedule(string ...$classes): Runner + { + $runner = $this->runner(); + + foreach ($classes as $class) { + $runner->schedule($this->definition($class)); + } + + return $runner; + } + + public function test_an_interval_task_takes_a_turn_when_its_timer_fires(): void + { + $this->schedule(SweepMessages::class); + + $this->loop->tick(3); + + $this->assertSame(3, SweepMessages::$turns); + } + + public function test_an_interval_task_is_armed_for_the_interval_it_declared(): void + { + $this->schedule(SweepMessages::class); + + $this->assertSame(10.0, $this->loop->timers[0]->getInterval()); + $this->assertTrue($this->loop->timers[0]->isPeriodic()); + } + + /** + * A task is a repeating chore, so nothing happens until the first interval + * has passed — unless it says otherwise. + */ + public function test_a_task_does_not_take_a_turn_merely_by_being_scheduled(): void + { + $this->schedule(SweepMessages::class); + + $this->assertSame(0, SweepMessages::$turns); + } + + /** + * Catching up on what expired while the bot was down cannot wait out the + * first interval — but it still waits for the bot to finish starting. + */ + public function test_a_task_that_runs_on_boot_takes_its_first_turn_on_the_next_tick(): void + { + $this->schedule(BootTask::class); + + $this->assertSame(0, BootTask::$turns); + + $this->loop->drainFutureTicks(); + + $this->assertSame(1, BootTask::$turns); + } + + public function test_a_disabled_task_is_left_out_of_the_schedule_entirely(): void + { + $scheduled = $this->runner()->schedule($this->definition(DisabledTask::class)); + + $this->loop->tick(); + $this->loop->drainFutureTicks(); + + $this->assertFalse($scheduled); + $this->assertSame([], $this->loop->timers); + $this->assertSame(0, DisabledTask::$turns); + } + + /** + * A timer fires again whether or not the last turn threw. Without + * containment the exception travels into the event loop and takes the + * process with it. + */ + public function test_a_task_that_throws_is_reported_and_keeps_its_place(): void + { + $this->schedule(FailingTask::class, SweepMessages::class); + + $this->loop->tick(2); + + $this->assertSame(2, SweepMessages::$turns); + $this->assertTrue($this->logger->has('the database went away')); + } + + public function test_a_failure_is_counted_against_the_task(): void + { + $runner = $this->schedule(FailingTask::class); + + $this->loop->tick(2); + + $stats = $runner->statsFor('FailingTask'); + + $this->assertNotNull($stats); + $this->assertSame(2, $stats->totalRuns); + $this->assertSame(2, $stats->failures); + $this->assertSame('the database went away', $stats->lastError); + $this->assertSame(0.0, $stats->getSuccessRate()); + } + + public function test_a_turn_that_worked_is_counted_too(): void + { + $runner = $this->schedule(SweepMessages::class); + + $this->loop->tick(3); + + $stats = $runner->statsFor('SweepMessages'); + + $this->assertNotNull($stats); + $this->assertSame(3, $stats->totalRuns); + $this->assertSame(3, $stats->successfulRuns); + $this->assertSame(100.0, $stats->getSuccessRate()); + } + + /** + * A task running every ten seconds would otherwise write eight thousand + * lines a day saying nothing happened. + */ + public function test_an_ordinary_turn_is_not_announced(): void + { + $this->schedule(SweepMessages::class); + + $this->loop->tick(3); + + $this->assertNotContains('info', $this->logger->levels); + $this->assertNotContains('error', $this->logger->levels); + } + + /** + * A task slower than its own interval must not be started alongside itself, + * or each turn makes the next one slower until nothing else gets a look in. + */ + public function test_a_task_still_busy_from_its_last_turn_is_skipped(): void + { + $this->schedule(SlowTask::class); + + $this->loop->tick(4); + + $this->assertSame(1, SlowTask::$started); + $this->assertTrue($this->logger->has('still busy')); + } + + public function test_a_task_that_catches_up_is_run_again(): void + { + $this->schedule(SlowTask::class); + + $this->loop->tick(); + SlowTask::$holding->resolve(null); + $this->loop->tick(); + + $this->assertSame(2, SlowTask::$started); + } + + public function test_a_cancelled_task_stops_taking_turns(): void + { + $runner = $this->schedule(SweepMessages::class); + + $this->assertTrue($runner->cancel('SweepMessages')); + + $this->loop->tick(3); + + $this->assertSame(0, SweepMessages::$turns); + $this->assertSame([], $runner->scheduled()); + } + + public function test_cancelling_something_that_was_never_scheduled_says_so(): void + { + $this->assertFalse($this->schedule(SweepMessages::class)->cancel('NoSuchTask')); + } + + public function test_every_task_can_be_cancelled_at_once(): void + { + $runner = $this->schedule(SweepMessages::class, FailingTask::class); + + $runner->cancelAll(); + $this->loop->tick(3); + + $this->assertSame(0, SweepMessages::$turns); + $this->assertSame([], $runner->scheduled()); + } + + /** + * Several chores on one class each get their own place in the schedule. + */ + public function test_tasks_declared_on_methods_are_scheduled_separately(): void + { + $reflector = new ClassReflector(Housekeeping::class); + $compiler = new TaskCompiler(); + $runner = $this->runner(); + + foreach (['sweepMessages', 'pruneStatistics'] as $methodName) { + $method = $reflector->getMethod($methodName); + + /** @var Task $attribute */ + $attribute = $method->getAttribute(Task::class); + + $runner->schedule($compiler->compileMethod($reflector, $method, $attribute)); + } + + $this->assertSame( + ['Housekeeping::sweepMessages', 'nightly-prune'], + $runner->scheduled(), + ); + } +} diff --git a/tests/Unit/TaskCompilerTest.php b/tests/Unit/TaskCompilerTest.php new file mode 100644 index 0000000..cbe5d3c --- /dev/null +++ b/tests/Unit/TaskCompilerTest.php @@ -0,0 +1,141 @@ +getAttribute(Task::class); + + return new TaskCompiler()->compileClass($reflector, $attribute); + } + + private function compileMethod(string $class, string $methodName): TaskDefinition + { + $reflector = new ClassReflector($class); + $method = $reflector->getMethod($methodName); + + /** @var Task $attribute */ + $attribute = $method->getAttribute(Task::class); + + return new TaskCompiler()->compileMethod($reflector, $method, $attribute); + } + + public function test_a_task_on_a_class_runs_through_its_invoke(): void + { + $definition = $this->compileClass(SweepMessages::class); + + $this->assertSame(SweepMessages::class, $definition->handler); + $this->assertSame('__invoke', $definition->method->getName()); + $this->assertSame(10, $definition->interval); + } + + public function test_a_task_on_a_method_runs_through_that_method(): void + { + $definition = $this->compileMethod(Housekeeping::class, 'sweepMessages'); + + $this->assertSame(Housekeeping::class, $definition->handler); + $this->assertSame('sweepMessages', $definition->method->getName()); + } + + /** + * A method name on its own would collide between classes, and two tasks + * sharing a name share their statistics and cannot be cancelled apart. + */ + public function test_a_method_task_is_named_after_its_class_as_well(): void + { + $this->assertSame( + 'Housekeeping::sweepMessages', + $this->compileMethod(Housekeeping::class, 'sweepMessages')->name, + ); + } + + public function test_a_class_task_is_named_after_its_class(): void + { + $this->assertSame('SweepMessages', $this->compileClass(SweepMessages::class)->name); + } + + public function test_a_name_given_in_the_attribute_wins(): void + { + $this->assertSame( + 'nightly-prune', + $this->compileMethod(Housekeeping::class, 'pruneStatistics')->name, + ); + } + + public function test_a_class_task_without_an_invoke_method_is_refused(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('should declare an __invoke method'); + + $this->compileClass(HandlerlessTask::class); + } + + /** + * An expression nobody can read should fail while the bot is starting and + * say which task it came from, not four hours later inside a timer. + */ + public function test_an_unreadable_cron_expression_is_refused_at_discovery(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('UnreadableCronTask'); + + $this->compileClass(UnreadableCronTask::class); + } + + public function test_a_task_must_say_when_it_runs(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('either an interval or a cron expression'); + + new Task(); + } + + public function test_a_task_cannot_be_scheduled_two_ways_at_once(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('cannot be given both'); + + new Task(interval: 60, cron: '@daily'); + } + + /** + * A zero interval asks the loop to run the task as fast as it can, which + * starves the gateway heartbeat and drops the connection. + */ + public function test_a_task_must_run_at_least_a_second_apart(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('at least one second'); + + new Task(interval: 0); + } + + public function test_an_interval_reads_as_something_a_person_can_check(): void + { + $this->assertSame('every 10 seconds', $this->compileClass(SweepMessages::class)->schedule()); + $this->assertSame('cron: @daily', $this->compileMethod(Housekeeping::class, 'pruneStatistics')->schedule()); + } +} diff --git a/tests/Unit/TasksDiscoveryTest.php b/tests/Unit/TasksDiscoveryTest.php new file mode 100644 index 0000000..da60fee --- /dev/null +++ b/tests/Unit/TasksDiscoveryTest.php @@ -0,0 +1,126 @@ +location = new DiscoveryLocation( + namespace: 'Tempcord\\Plugins\\Tasks\\Tests\\Fixtures\\', + path: dirname(__DIR__) . '/Fixtures', + ); + } + + private function registry(): Registry + { + return new Registry(new GenericContainer()); + } + + private function discover(Registry $registry, string ...$classes): void + { + $discovery = new TasksDiscovery($registry); + $discovery->setItems(new DiscoveryItems()); + + foreach ($classes as $class) { + $discovery->discover($this->location, new ClassReflector($class)); + } + + $discovery->apply(); + } + + /** + * @return list + */ + private function names(Registry $registry): array + { + $names = array_map(static fn($task) => $task->name, $registry->all()); + sort($names); + + return $names; + } + + public function test_it_finds_every_annotated_method(): void + { + $registry = $this->registry(); + $this->discover($registry, ScheduledCommands::class); + + $this->assertSame( + ['ScheduledCommands::disabled', 'ScheduledCommands::everyMinute', 'hourly-report'], + $this->names($registry), + ); + } + + /** + * A task on the class itself is found the same way, so a bot may declare + * one task per class the way it declares commands and listeners. + */ + public function test_it_finds_a_task_declared_on_the_class(): void + { + $registry = $this->registry(); + $this->discover($registry, SweepMessages::class); + + $this->assertSame(['SweepMessages'], $this->names($registry)); + } + + /** + * A method without the attribute is left alone, so helpers can sit beside + * scheduled work. + */ + public function test_it_ignores_methods_without_the_attribute(): void + { + $registry = $this->registry(); + $this->discover($registry, ScheduledCommands::class); + + $this->assertNotContains('ScheduledCommands::notATask', $this->names($registry)); + } + + /** + * The method is what the runner calls later, so discovery has to carry it + * through. + */ + public function test_each_task_knows_what_to_call(): void + { + $registry = $this->registry(); + $this->discover($registry, ScheduledCommands::class); + + foreach ($registry->all() as $task) { + $this->assertSame(ScheduledCommands::class, $task->handler); + $this->assertSame( + ScheduledCommands::class, + $task->method->getDeclaringClass()->getName(), + ); + } + } + + /** + * Registration happens in apply, not while discovering, so a discovery run + * that is thrown away leaves no trace. + */ + public function test_nothing_is_registered_until_apply(): void + { + $registry = $this->registry(); + + $discovery = new TasksDiscovery($registry); + $discovery->setItems(new DiscoveryItems()); + $discovery->discover($this->location, new ClassReflector(ScheduledCommands::class)); + + $this->assertSame(0, $registry->count()); + } +} diff --git a/tests/Unit/TasksPluginTest.php b/tests/Unit/TasksPluginTest.php new file mode 100644 index 0000000..d2d9c53 --- /dev/null +++ b/tests/Unit/TasksPluginTest.php @@ -0,0 +1,52 @@ +singleton(Logger::class, new RecordingLogger()); + + $plugin = new TasksPlugin(new Registry($container), new RecordingLogger()); + + $this->assertInstanceOf(Plugin::class, $plugin); + } + + /** + * A bot that installed the plugin but declared no tasks should say nothing + * on the way up. + */ + public function test_a_bot_with_no_tasks_is_not_told_about_it(): void + { + $container = new GenericContainer(); + $logger = new RecordingLogger(); + $container->singleton(Logger::class, $logger); + + // Tempcord is final, and boot() only needs something of that type. + $tempcord = new ReflectionClass(Tempcord::class)->newInstanceWithoutConstructor(); + + new TasksPlugin(new Registry($container), $logger)->boot($tempcord); + + $this->assertSame([], $logger->messages); + } +}