Skip to content
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.

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

Reintroduce support for PHP 7.1 and 7.2 #4386

Merged
merged 1 commit into from
Mar 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,34 @@ jobs:
name: "phpunit-sqlite-${{ matrix.deps }}-${{ matrix.php-version }}.coverage"
path: "coverage.xml"

phpunit-lower-php-versions:
name: "PHPUnit with SQLite"
runs-on: "ubuntu-20.04"

strategy:
matrix:
php-version:
- "7.1"
- "7.2"

steps:
- name: "Checkout"
uses: "actions/checkout@v2"
with:
fetch-depth: 2

- name: "Install PHP"
uses: "shivammathur/setup-php@v2"
with:
php-version: "${{ matrix.php-version }}"
ini-values: "zend.assertions=1"

- name: "Install dependencies with Composer"
uses: "ramsey/composer-install@v1"

- name: "Run PHPUnit"
run: "vendor/bin/phpunit -c ci/github/phpunit/sqlite.xml"

phpunit-oci8:
name: "PHPUnit on OCI8"
runs-on: "ubuntu-20.04"
Expand Down
11 changes: 4 additions & 7 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
{"name": "Jonathan Wage", "email": "jonwage@gmail.com"}
],
"require": {
"php": "^7.3 || ^8",
"php": "^7.1 || ^8",
"ext-pdo": "*",
"doctrine/cache": "^1.0",
"doctrine/event-manager": "^1.0"
Expand All @@ -41,8 +41,8 @@
"doctrine/coding-standard": "8.2.0",
"jetbrains/phpstorm-stubs": "2019.3",
"phpstan/phpstan": "0.12.80",
"phpunit/phpunit": "9.5.0",
"psalm/plugin-phpunit": "0.13.0",
"phpunit/phpunit": "^7.5.20|^8.5|9.5.0",
"psalm/plugin-phpunit": "0.14.0",
"symfony/console": "^2.0.5|^3.0|^4.0|^5.0",
"vimeo/psalm": "4.5.2"
},
Expand All @@ -51,10 +51,7 @@
},
"bin": ["bin/doctrine-dbal"],
"config": {
"sort-packages": true,
"platform": {
"php": "7.3.0"
Copy link
Member

Choose a reason for hiding this comment

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

W/o the platform fixed, all dependency updates will have to be made on the lowest supported version. Otherwise, there's a risk of locking the dependencies that are not compatible with it.

It might be better to unset the platform version on CI before the update but have it committed in the repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

When supporting this range of versions, you need to "composer update" anyways to get to the right packages. So the solution is probably rather to remove composer.lock in this step. With this setting in, you cannot get the code running locally with 7.1 and 7.2

Copy link
Member

Choose a reason for hiding this comment

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

But without this setting, the one performing a dependency update will have to use PHP 7.1. Otherwise, they will learn that the new resolution doesn't work with PHP 7.1 only on CI. Is there a Composer CLI switch to pass config.platform.php at runtime?

Copy link
Member Author

Choose a reason for hiding this comment

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

This point is not relevant anymore since ce3421e composer.lock is not committed anymore and the Ci always fetches the latest packeges matching all constraints.

}
"sort-packages": true
},
"autoload": {
"psr-4": { "Doctrine\\DBAL\\": "lib/Doctrine/DBAL" }
Expand Down
15 changes: 6 additions & 9 deletions lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,12 @@ public function getDropIndexSQL($index, $table = null)
}

return sprintf(
<<<SQL
"
IF EXISTS (SELECT * FROM sysobjects WHERE name = '%s')
ALTER TABLE %s DROP CONSTRAINT %s
ELSE
DROP INDEX %s ON %s
SQL
,
",
$index,
$table,
$index,
Expand Down Expand Up @@ -1691,12 +1690,11 @@ private function generateIdentifierName($identifier)
protected function getCommentOnTableSQL(string $tableName, ?string $comment): string
{
return sprintf(
<<<'SQL'
"
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

HEREDOCs were added in PHP 7.3

Copy link
Member Author

Choose a reason for hiding this comment

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

Correction, moving the closing tag for now/heredocs to the same indentation as starting was added in 7.3

Copy link
Member

Choose a reason for hiding this comment

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

Won't moving the heredoc terminator to the beginning of the line have more or less the same effect?

Copy link
Contributor

@simonberger simonberger Mar 7, 2021

Choose a reason for hiding this comment

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

It applies a tab more for every line, but works.
imho it's better to not indent heredoc blocks.

return sprintf(
    <<<SQL
EXEC sys.sp_addextendedproperty @name=N'MS_Description',
    @value=N%s, @level0type=N'SCHEMA', @level0name=N'dbo',
@level1type=N'TABLE', @level1name=N%s
SQL,
    $this->quoteStringLiteral((string) $comment),
    $this->quoteStringLiteral($tableName)
);

Copy link
Member

@greg0ire greg0ire Mar 23, 2021

Choose a reason for hiding this comment

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

@beberlei you could use git restore --source=origin/2.12.x lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php && git revert 9a8ea5e039268368d48f38c853a00840e8201bde to make it 7.1-compatible, that way syntactic coloration is kept.

EXEC sys.sp_addextendedproperty @name=N'MS_Description',
@value=N%s, @level0type=N'SCHEMA', @level0name=N'dbo',
@level1type=N'TABLE', @level1name=N%s
SQL
,
",
$this->quoteStringLiteral((string) $comment),
$this->quoteStringLiteral($tableName)
);
Expand All @@ -1705,16 +1703,15 @@ protected function getCommentOnTableSQL(string $tableName, ?string $comment): st
public function getListTableMetadataSQL(string $table): string
{
return sprintf(
<<<'SQL'
"
SELECT
p.value AS [table_comment]
FROM
sys.tables AS tbl
INNER JOIN sys.extended_properties AS p ON p.major_id=tbl.object_id AND p.minor_id=0 AND p.class=1
WHERE
(tbl.name=N%s and SCHEMA_NAME(tbl.schema_id)=N'dbo' and p.name=N'MS_Description')
SQL
,
",
$this->quoteStringLiteral($table)
);
}
Expand Down
41 changes: 41 additions & 0 deletions tests/Doctrine/Tests/DBAL/AssertionCompatibility.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Doctrine\Tests\DBAL;

trait AssertionCompatibility
{
/**
* @param array<mixed> $arguments
*
* @return mixed
*/
public static function __callStatic(string $name, array $arguments)
{
if ($name === 'assertMatchesRegularExpression') {
self::assertRegExp(...$arguments);
} elseif ($name === 'assertFileDoesNotExist') {
self::assertFileNotExists(...$arguments);
}

return null;
}

/**
* @param array<mixed> $arguments
*
* @return mixed
*/
public function __call(string $method, array $arguments)
{
if ($method === 'createStub') {
return $this->getMockBuilder(...$arguments)
->disableOriginalConstructor()
->disableOriginalClone()
->disableArgumentCloning()
->disallowMockingUnknownTypes()
->getMock();
}

return null;
}
}
3 changes: 3 additions & 0 deletions tests/Doctrine/Tests/DBAL/Connection/LoggingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@
use Doctrine\DBAL\Driver\Statement;
use Doctrine\DBAL\Logging\SQLLogger;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\Tests\DBAL\AssertionCompatibility;
use PHPUnit\Framework\TestCase;

final class LoggingTest extends TestCase
{
use AssertionCompatibility;

public function testLogExecuteQuery(): void
{
$driverConnection = $this->createStub(DriverConnection::class);
Expand Down
14 changes: 7 additions & 7 deletions tests/Doctrine/Tests/DBAL/ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private function getExecuteStatementMockConnection()

$platform = $this->getMockForAbstractClass(AbstractPlatform::class);

return $this->getMockBuilder(Connection::class)
return (new MockBuilderProxy($this->getMockBuilder(Connection::class)))
->onlyMethods(['executeStatement'])
->setConstructorArgs([['platform' => $platform], $driverMock])
->getMock();
Expand Down Expand Up @@ -528,7 +528,7 @@ public function testFetchAssoc(): void
->with(FetchMode::ASSOCIATIVE)
->will($this->returnValue($result));

$conn = $this->getMockBuilder(Connection::class)
$conn = (new MockBuilderProxy($this->getMockBuilder(Connection::class)))
->onlyMethods(['executeQuery'])
->setConstructorArgs([[], $driverMock])
->getMock();
Expand Down Expand Up @@ -563,7 +563,7 @@ public function testFetchArray(): void
->with(FetchMode::NUMERIC)
->will($this->returnValue($result));

$conn = $this->getMockBuilder(Connection::class)
$conn = (new MockBuilderProxy($this->getMockBuilder(Connection::class)))
->onlyMethods(['executeQuery'])
->setConstructorArgs([[], $driverMock])
->getMock();
Expand Down Expand Up @@ -599,7 +599,7 @@ public function testFetchColumn(): void
->with($column)
->will($this->returnValue($result));

$conn = $this->getMockBuilder(Connection::class)
$conn = (new MockBuilderProxy($this->getMockBuilder(Connection::class)))
->onlyMethods(['executeQuery'])
->setConstructorArgs([[], $driverMock])
->getMock();
Expand Down Expand Up @@ -633,7 +633,7 @@ public function testFetchAll(): void
->method('fetchAll')
->will($this->returnValue($result));

$conn = $this->getMockBuilder(Connection::class)
$conn = (new MockBuilderProxy($this->getMockBuilder(Connection::class)))
->onlyMethods(['executeQuery'])
->setConstructorArgs([[], $driverMock])
->getMock();
Expand Down Expand Up @@ -709,9 +709,9 @@ public function testCallConnectOnce(string $method, array $params): void
->method('prepare')
->will($this->returnValue($stmtMock));

$conn = $this->getMockBuilder(Connection::class)
->setConstructorArgs([['pdo' => $pdoMock, 'platform' => $platformMock], $driverMock])
$conn = (new MockBuilderProxy($this->getMockBuilder(Connection::class)))
->onlyMethods(['connect'])
->setConstructorArgs([['pdo' => $pdoMock, 'platform' => $platformMock], $driverMock])
->getMock();

$conn->expects($this->once())->method('connect');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@

use Doctrine\DBAL\Connections\MasterSlaveConnection;
use Doctrine\DBAL\Driver;
use Doctrine\Tests\DBAL\AssertionCompatibility;
use Doctrine\Tests\DbalTestCase;

class MasterSlaveConnectionTest extends DbalTestCase
{
use AssertionCompatibility;

public function testConnectionParamsRemainAvailable(): void
{
$constructionParams = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public function testExecute(array $params): void
->withConsecutive(
[1, $params[0]],
[2, $params[1]],
[3, $params[2]],
[3, $params[2]]
);

// the return value is irrelevant to the test
Expand Down
5 changes: 3 additions & 2 deletions tests/Doctrine/Tests/DBAL/Functional/ExceptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@
use function posix_geteuid;
use function posix_getpwuid;
use function sprintf;
use function strtoupper;
use function substr;
use function sys_get_temp_dir;
use function touch;
use function unlink;
use function version_compare;

use const PHP_OS;
use const PHP_OS_FAMILY;

/**
* @psalm-import-type Params from DriverManager
Expand Down Expand Up @@ -306,7 +307,7 @@ public function testConnectionExceptionSqLite(): void
}

// mode 0 is considered read-only on Windows
$mode = PHP_OS_FAMILY === 'Windows' ? 0000 : 0444;
$mode = strtoupper(substr(PHP_OS, 0, 3)) === 'WIN' ? 0000 : 0444;

$filename = sprintf('%s/%s', sys_get_temp_dir(), 'doctrine_failed_connection_' . $mode . '.db');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ public function testColumnComments(): void
'commented_req_change_column',
Type::getType('integer'),
['comment' => 'Some comment', 'notnull' => false]
),
)
);

$tableDiff->removedColumns['comment_integer_0']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
use Doctrine\DBAL\Types\BlobType;
use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Types\Types;
use Doctrine\Tests\DBAL\AssertionCompatibility;

use function dirname;

class SqliteSchemaManagerTest extends SchemaManagerFunctionalTestCase
{
use AssertionCompatibility;

protected function supportsPlatform(AbstractPlatform $platform): bool
{
return $platform instanceof SqlitePlatform;
Expand Down
38 changes: 38 additions & 0 deletions tests/Doctrine/Tests/DBAL/MockBuilderProxy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php

namespace Doctrine\Tests\DBAL;

use PHPUnit\Framework\MockObject\MockBuilder;

use function method_exists;

/**
* @template T
*/
class MockBuilderProxy
{
/** @var MockBuilder<T> */
private $originalMockBuilder;

/**
* @param MockBuilder<T> $originalMockBuilder
*/
public function __construct(MockBuilder $originalMockBuilder)
{
$this->originalMockBuilder = $originalMockBuilder;
}

/**
* @param array<string> $methods
*
* @return MockBuilder<T>
*/
public function onlyMethods(array $methods): MockBuilder
{
if (method_exists(MockBuilder::class, 'onlyMethods')) {
return $this->originalMockBuilder->onlyMethods($methods);
}

return $this->originalMockBuilder->setMethods($methods);
}
}
5 changes: 3 additions & 2 deletions tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\Types\Type;
use Doctrine\Tests\DBAL\MockBuilderProxy;
use PHPUnit\Framework\TestCase;

use function array_keys;
Expand Down Expand Up @@ -1163,10 +1164,10 @@ public function testComplexDiffColumn(): void
public function testComparesNamespaces(): void
{
$comparator = new Comparator();
$fromSchema = $this->getMockBuilder(Schema::class)
$fromSchema = (new MockBuilderProxy($this->getMockBuilder(Schema::class)))
->onlyMethods(['getNamespaces', 'hasNamespace'])
->getMock();
$toSchema = $this->getMockBuilder(Schema::class)
$toSchema = (new MockBuilderProxy($this->getMockBuilder(Schema::class)))
->onlyMethods(['getNamespaces', 'hasNamespace'])
->getMock();

Expand Down
5 changes: 3 additions & 2 deletions tests/Doctrine/Tests/DBAL/Schema/DB2SchemaManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Doctrine\DBAL\Driver;
use Doctrine\DBAL\Platforms\DB2Platform;
use Doctrine\DBAL\Schema\DB2SchemaManager;
use Doctrine\Tests\DBAL\MockBuilderProxy;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;

Expand All @@ -29,8 +30,8 @@ protected function setUp(): void
$eventManager = new EventManager();
$driverMock = $this->createMock(Driver::class);
$platform = $this->createMock(DB2Platform::class);
$this->conn = $this
->getMockBuilder(Connection::class)
$this->conn = (new MockBuilderProxy($this
->getMockBuilder(Connection::class)))
->onlyMethods(['fetchAllAssociative', 'quote'])
->setConstructorArgs([['platform' => $platform], $driverMock, new Configuration(), $eventManager])
->getMock();
Expand Down
3 changes: 2 additions & 1 deletion tests/Doctrine/Tests/DBAL/Schema/MySqlSchemaManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Doctrine\DBAL\Schema\AbstractSchemaManager;
use Doctrine\DBAL\Schema\ForeignKeyConstraint;
use Doctrine\DBAL\Schema\MySqlSchemaManager;
use Doctrine\Tests\DBAL\MockBuilderProxy;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;

Expand All @@ -32,7 +33,7 @@ protected function setUp(): void
$platform->method('getListTableForeignKeysSQL')
->willReturn('');

$this->conn = $this->getMockBuilder(Connection::class)
$this->conn = (new MockBuilderProxy($this->getMockBuilder(Connection::class)))
->onlyMethods(['fetchAllAssociative'])
->setConstructorArgs([['platform' => $platform], $driverMock, new Configuration(), $eventManager])
->getMock();
Expand Down
Loading