Skip to content

Add symfony 6 comptability #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 25, 2022
Merged

Add symfony 6 comptability #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e870f1f
Add symfony 6 comptability
Prokyonn Jul 22, 2022
dea1565
Fix doctrine storage
Prokyonn Jul 25, 2022
50b894e
Fix unit test
Prokyonn Jul 25, 2022
70050bb
Update php-task version
Prokyonn Jul 25, 2022
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
56 changes: 11 additions & 45 deletions .github/workflows/test-application.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,57 +21,23 @@ jobs:
fail-fast: false
matrix:
include:
- php-version: '5.6'
dependencies: 'lowest'
storage: doctrine
- php-version: '5.6'
storage: doctrine
- php-version: '5.6'
storage: array

- php-version: '7.0'
dependencies: 'lowest'
storage: doctrine
- php-version: '7.0'
storage: doctrine
- php-version: '7.0'
storage: array

- php-version: '7.1'
dependencies: 'lowest'
storage: doctrine
- php-version: '7.1'
storage: doctrine
- php-version: '7.1'
storage: array

- php-version: '7.2'
dependencies: 'lowest'
storage: doctrine
- php-version: '7.2'
storage: doctrine
- php-version: '7.2'
storage: array

- php-version: '7.3'
storage: doctrine
- php-version: '7.3'
storage: array

- php-version: '7.4'
- php-version: '8.0'
coverage: '--coverage-clover=coverage.clover'
storage: doctrine
- php-version: '7.4'
- php-version: '8.0'
storage: array

- php-version: '8.0'
- php-version: '8.1'
storage: doctrine
- php-version: '8.0'
- php-version: '8.1'
storage: array

steps:
- name: Checkout project
uses: actions/checkout@v2
with:
# Fetch 10 commits or Scrutinizer will throw ("Failed to retrieve commit parents. If you use a shallow git checkout, please checkout at least a depth of one."), see: RepositoryIntrospector at scrutinizer-ci/ocular GitHub repository
# 10 commits is an arbitrary value that is more than 1 commit
fetch-depth: 10

- name: Install and configure PHP
uses: shivammathur/setup-php@v2
Expand All @@ -82,7 +48,7 @@ jobs:
tools: 'composer:v2'

- name: Install dependencies with Composer
uses: ramsey/composer-install@v1
uses: ramsey/composer-install@v2
with:
dependency-versions: ${{ matrix.dependencies }}
composer-options: --prefer-dist --no-suggest
Expand All @@ -99,5 +65,5 @@ jobs:
- name: Coverage
if: matrix.coverage
run: |
wget https://scrutinizer-ci.com/ocular.phar
php ocular.phar code-coverage:upload --access-token="230ec5e01daf5bb3e46ea304fb20348b52d80de73463ec08ee9c96fcd1349e35" --format=php-clover coverage.clover
composer global require scrutinizer/ocular
~/.composer/vendor/bin/ocular code-coverage:upload --access-token="230ec5e01daf5bb3e46ea304fb20348b52d80de73463ec08ee9c96fcd1349e35" --format=php-clover coverage.clover
29 changes: 14 additions & 15 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,23 @@
}
],
"require": {
"php": "^5.6 || ^7.0 || ^8.0",
"php-task/php-task": "^1.4",
"symfony/http-kernel": "^2.8 || ^3.4 || ^4.0 || ^5.0",
"symfony/dependency-injection": "^2.8 || ^3.4 || ^4.0 || ^5.0",
"symfony/expression-language": "^2.8 || ^3.4 || ^4.0 || ^5.0",
"symfony/config": "^2.8 || ^3.4 || ^4.0 || ^5.0",
"symfony/console": "^2.8 || ^3.4 || ^4.0 || ^5.0",
"symfony/process": "^2.8 || ^3.4 || ^4.0 || ^5.0",
"doctrine/orm": "^2.5"
"php": "^8.0 || ^8.1",
"php-task/php-task": "^2.0",
"symfony/http-kernel": "^5.4 || ^6.0",
"symfony/dependency-injection": "^5.4 || ^6.0",
"symfony/expression-language": "^5.4 || ^6.0",
"symfony/config": "^5.4 || ^6.0",
"symfony/console": "^5.4 || ^6.0",
"symfony/process": "^5.4 || ^6.0",
"doctrine/orm": "^2.5.3"
},
"require-dev": {
"symfony/debug": "^2.8 || ^3.4 || ^4.0 || ^5.0",
"symfony/framework-bundle": "^2.8.18 || ^3.4 || ^4.0 || ^5.0",
"symfony/finder": "^2.8 || ^3.4 || ^4.0 || ^5.0",
"symfony/yaml": "^2.8 || ^3.4 || ^4.0 || ^5.0",
"symfony/phpunit-bridge": "^5.2.3",
"symfony/framework-bundle": "^5.4 || ^6.0",
"symfony/finder": "^5.4 || ^6.0",
"symfony/yaml": "^5.4 || ^6.0",
"symfony/phpunit-bridge": "^5.4 || ^6.0",
"doctrine/doctrine-bundle": "^1.5 || ^2.0",
"doctrine/data-fixtures": "^1.1"
"doctrine/data-fixtures": "^1.3.3"
},
"autoload": {
"psr-4": {
Expand Down
19 changes: 6 additions & 13 deletions src/Command/ExecuteCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use Symfony\Component\Console\Output\ConsoleOutputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\LegacyEventDispatcherProxy;
use Task\Event\Events;
use Task\Event\TaskEvent;
use Task\Event\TaskExecutionEvent;
Expand Down Expand Up @@ -82,9 +81,12 @@ protected function execute(InputInterface $input, OutputInterface $output)
$handler = $this->handlerFactory->create($execution->getHandlerClass());

try {
$this->dispatch(Events::TASK_BEFORE, new TaskEvent($execution->getTask()));
$this->eventDispatcher->dispatch(new TaskEvent($execution->getTask()), Events::TASK_BEFORE);
$result = $handler->handle($execution->getWorkload());
$this->dispatch(Events::TASK_AFTER, new TaskExecutionEvent($execution->getTask(), $execution));
$this->eventDispatcher->dispatch(
new TaskExecutionEvent($execution->getTask(), $execution),
Events::TASK_AFTER
);
} catch (\Exception $exception) {
if ($exception instanceof FailedException) {
$errorOutput->writeln(FailedException::class);
Expand All @@ -105,17 +107,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
/**
* {@inheritdoc}
*/
public function isHidden()
public function isHidden(): bool
{
return true;
}

private function dispatch($eventName, $event)
{
if (class_exists(LegacyEventDispatcherProxy::class)) {
return $this->eventDispatcher->dispatch($event, $eventName);
} else {
return $this->eventDispatcher->dispatch($eventName, $event);
}
}
}
4 changes: 2 additions & 2 deletions src/Resources/config/storage/doctrine.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
<service id="task.repository.task" class="Task\TaskBundle\Entity\TaskRepository" public="true">
<factory service="doctrine.orm.entity_manager" method="getRepository"/>

<argument type="string">TaskBundle:Task</argument>
<argument type="string">Task\TaskBundle\Entity\Task</argument>
</service>
<service id="task.storage.task" alias="task.repository.task" public="true"/>

<service id="task.repository.task_execution" class="Task\TaskBundle\Entity\TaskExecutionRepository" public="true">
<factory service="doctrine.orm.entity_manager" method="getRepository"/>

<argument type="string">TaskBundle:TaskExecution</argument>
<argument type="string">Task\TaskBundle\Entity\TaskExecution</argument>
</service>
<service id="task.storage.task_execution" alias="task.repository.task_execution" public="true"/>

Expand Down
16 changes: 12 additions & 4 deletions tests/Functional/Command/RunCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function testExecute()
{
$singleTask = $this->createTask('Test workload 1');
$laterTask = $this->createTask('Test workload 2');
$intervalTask = $this->createTask('Test workload 3', CronExpression::factory('@daily'));
$intervalTask = $this->createTask('Test workload 3', new CronExpression('@daily'));

/** @var TaskExecutionInterface[] $executions */
$executions = [
Expand Down Expand Up @@ -70,7 +70,11 @@ public function testExecute()
$this->assertEquals($intervalTask->getWorkload(), $task->getWorkload());
$this->assertLessThanOrEqual($intervalTask->getFirstExecution(), $task->getFirstExecution());
$this->assertLessThanOrEqual($intervalTask->getLastExecution(), $task->getLastExecution());
$this->assertEquals($intervalTask->getInterval(), $task->getInterval());

$this->assertEquals($intervalTask->getInterval()->getExpression(), $task->getInterval()->getExpression());
$this->assertEquals($intervalTask->getInterval()->getNextRunDate(), $task->getInterval()->getNextRunDate());
$this->assertEquals($intervalTask->getInterval()->getParts(), $task->getInterval()->getParts());
$this->assertEquals($intervalTask->getInterval()->getPreviousRunDate(), $task->getInterval()->getPreviousRunDate());

$this->assertEquals(TaskStatus::PLANNED, $result[0]->getStatus());
$this->assertEquals(TestHandler::class, $result[0]->getHandlerClass());
Expand All @@ -81,7 +85,7 @@ public function testExecuteWithFail()
{
$singleTask = $this->createTask('Test workload 1', null, FailTestHandler::class);
$laterTask = $this->createTask('Test workload 2', null, FailTestHandler::class);
$intervalTask = $this->createTask('Test workload 3', CronExpression::factory('@daily'), FailTestHandler::class);
$intervalTask = $this->createTask('Test workload 3', new CronExpression('@daily'), FailTestHandler::class);

/** @var TaskExecutionInterface[] $executions */
$executions = [
Expand Down Expand Up @@ -124,7 +128,11 @@ public function testExecuteWithFail()
$this->assertEquals($intervalTask->getWorkload(), $task->getWorkload());
$this->assertLessThanOrEqual($intervalTask->getFirstExecution(), $task->getFirstExecution());
$this->assertLessThanOrEqual($intervalTask->getLastExecution(), $task->getLastExecution());
$this->assertEquals($intervalTask->getInterval(), $task->getInterval());
Copy link
Contributor Author

@Prokyonn Prokyonn Jul 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

asserting equals between the interval object resulted in an error: https://github.com/php-task/TaskBundle/runs/7495173211?check_suite_focus=true
honestly I've no idea why this happens in the CI, locally everything works as expected 😅

but I guess the new additional lines test the same thing and it is working now also in the CI :)

Copy link
Member

Choose a reason for hiding this comment

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

@Prokyonn is there a __toString method? i would compare at least this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wachterjohannes https://github.com/dragonmantank/cron-expression/blob/master/src/Cron/CronExpression.php#L384-L387 yes, but this is already implicitly tested in line 132:

$this->assertEquals($intervalTask->getInterval()->getExpression(), $task->getInterval()->getExpression());

Copy link
Member

Choose a reason for hiding this comment

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

OK then all right - should we tag the library first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah makes sense to add a version instead of the dev branch before merging :)


$this->assertEquals($intervalTask->getInterval()->getExpression(), $task->getInterval()->getExpression());
$this->assertEquals($intervalTask->getInterval()->getNextRunDate(), $task->getInterval()->getNextRunDate());
$this->assertEquals($intervalTask->getInterval()->getParts(), $task->getInterval()->getParts());
$this->assertEquals($intervalTask->getInterval()->getPreviousRunDate(), $task->getInterval()->getPreviousRunDate());

$this->assertEquals(TaskStatus::PLANNED, $result[0]->getStatus());
$this->assertEquals(FailTestHandler::class, $result[0]->getHandlerClass());
Expand Down
14 changes: 7 additions & 7 deletions tests/Unit/Command/ScheduleSystemTasksCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use Cron\CronExpression;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\StringInput;
use Symfony\Component\Console\Output\OutputInterface;
use Task\Execution\TaskExecutionInterface;
use Task\Scheduler\TaskSchedulerInterface;
Expand Down Expand Up @@ -85,7 +85,7 @@ public function testExecute()
$taskBuilder->schedule()->shouldBeCalled();

$command->run(
$this->prophesize(InputInterface::class)->reveal(),
new StringInput(''),
$this->prophesize(OutputInterface::class)->reveal()
);
}
Expand Down Expand Up @@ -135,7 +135,7 @@ public function testExecuteMultiple()
$taskBuilder2->schedule()->shouldBeCalled();

$command->run(
$this->prophesize(InputInterface::class)->reveal(),
new StringInput(''),
$this->prophesize(OutputInterface::class)->reveal()
);
}
Expand Down Expand Up @@ -178,7 +178,7 @@ function ($date) {
$this->taskExecutionRepository->save($execution->reveal())->shouldBeCalled();

$command->run(
$this->prophesize(InputInterface::class)->reveal(),
new StringInput(''),
$this->prophesize(OutputInterface::class)->reveal()
);
}
Expand Down Expand Up @@ -218,7 +218,7 @@ public function testExecuteUpdate()
$this->scheduler->scheduleTasks()->shouldBeCalled();

$command->run(
$this->prophesize(InputInterface::class)->reveal(),
new StringInput(''),
$this->prophesize(OutputInterface::class)->reveal()
);
}
Expand Down Expand Up @@ -253,7 +253,7 @@ public function testExecuteUpdateNotSupported()
$this->scheduler->scheduleTasks()->shouldNotBeCalled();

$command->run(
$this->prophesize(InputInterface::class)->reveal(),
new StringInput(''),
$this->prophesize(OutputInterface::class)->reveal()
);
}
Expand Down Expand Up @@ -287,7 +287,7 @@ function ($date) {
$this->taskExecutionRepository->save($execution->reveal())->shouldBeCalled();

$command->run(
$this->prophesize(InputInterface::class)->reveal(),
new StringInput(''),
$this->prophesize(OutputInterface::class)->reveal()
);
}
Expand Down
15 changes: 9 additions & 6 deletions tests/Unit/DependencyInjection/HandlerCompilerPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ public function testProcess()
$serviceDefinition = $this->prophesize(Definition::class);
$container->findDefinition(HandlerCompilerPass::REGISTRY_ID)->willReturn($serviceDefinition);

$compilerPass = new HandlerCompilerPass();
$compilerPass->process($container->reveal());

$serviceDefinition->replaceArgument(
0,
[\stdClass::class => new Reference('service1'), self::class => new Reference('service2')]
)->shouldBeCalled();
)->shouldBeCalled()
->willReturn($serviceDefinition->reveal());

$compilerPass = new HandlerCompilerPass();
$compilerPass->process($container->reveal());
}

public function testProcessNoTaggedService()
Expand All @@ -55,9 +56,11 @@ public function testProcessNoTaggedService()
$serviceDefinition = $this->prophesize(Definition::class);
$container->findDefinition(HandlerCompilerPass::REGISTRY_ID)->willReturn($serviceDefinition);

$serviceDefinition->replaceArgument(0, [])
->shouldBeCalled()
->willReturn($serviceDefinition->reveal());

$compilerPass = new HandlerCompilerPass();
$compilerPass->process($container->reveal());

$serviceDefinition->replaceArgument(0, [])->shouldBeCalled();
}
}
13 changes: 7 additions & 6 deletions tests/app/TestKernel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\HttpKernel\Kernel;
use Task\TaskBundle\TaskBundle;
Expand All @@ -29,7 +30,7 @@ class TestKernel extends Kernel
/**
* {@inheritdoc}
*/
public function registerBundles()
public function registerBundles(): array
{
return [
new FrameworkBundle(),
Expand All @@ -41,7 +42,7 @@ public function registerBundles()
/**
* {@inheritdoc}
*/
public function registerContainerConfiguration(LoaderInterface $loader)
public function registerContainerConfiguration(LoaderInterface $loader): void
{
$this->storage = getenv(self::STORAGE_VAR_NAME);
if (false === $this->storage) {
Expand All @@ -55,7 +56,7 @@ public function registerContainerConfiguration(LoaderInterface $loader)
/**
* {@inheritdoc}
*/
protected function buildContainer()
protected function buildContainer(): ContainerBuilder
{
$container = parent::buildContainer();
$loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/config'));
Expand All @@ -70,7 +71,7 @@ protected function buildContainer()
/**
* {@inheritdoc}
*/
protected function initializeContainer()
protected function initializeContainer(): void
{
static $first = true;

Expand All @@ -92,7 +93,7 @@ protected function initializeContainer()

try {
parent::initializeContainer();
} catch (\Exception $e) {
} catch (Exception $e) {
$this->debug = $debug;

throw $e;
Expand All @@ -101,7 +102,7 @@ protected function initializeContainer()
$this->debug = $debug;
}

protected function getKernelParameters()
protected function getKernelParameters(): array
{
return array_merge(
parent::getKernelParameters(),
Expand Down
2 changes: 1 addition & 1 deletion tests/app/console
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<?php
use Symfony\Bundle\FrameworkBundle\Console\Application;
use Symfony\Component\Console\Input\ArgvInput;
use Symfony\Component\Debug\Debug;
use Symfony\Component\ErrorHandler\Debug;

// if you don't want to setup permissions the proper way, just uncomment the following PHP line
// read http://symfony.com/doc/current/book/installation.html#configuration-and-setup for more information
Expand Down