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

Fix various warnings on PHP 8.2 / PHPUnit 9 #340

Merged
merged 9 commits into from
Mar 22, 2024
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
12 changes: 6 additions & 6 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion doc/develop.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ For example:
```bash
export CIVIX_WORKSPACE=$HOME/buildkit/build/dmaster/web/sites/all/modules/civicrm/ext/civixtest
civibuild restore dmaster
phpunit8
phpunit9
```

The helper `./scripts/run-tests.sh` is a small wrapper which does the above.
Expand Down
28 changes: 14 additions & 14 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<phpunit backupGlobals="false"
<?xml version="1.0"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
backupGlobals="false"
backupStaticAttributes="false"
colors="true"
convertErrorsToExceptions="true"
Expand All @@ -9,7 +11,17 @@
stopOnFailure="false"
bootstrap="tests/bootstrap.php"
cacheResult="false"
>
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
<coverage>
<include>
<directory suffix=".php">./src</directory>
</include>
<exclude>
<directory>src/*Bundle/Resources</directory>
<directory>src/*/*Bundle/Resources</directory>
<directory>src/*/Bundle/*Bundle/Resources</directory>
</exclude>
</coverage>
<testsuites>
<testsuite name="unit">
<directory>./src/</directory>
Expand All @@ -18,18 +30,6 @@
<directory>./tests/e2e/</directory>
</testsuite>
</testsuites>

<filter>
<whitelist>
<directory suffix=".php">./src</directory>
<exclude>
<directory>src/*Bundle/Resources</directory>
<directory>src/*/*Bundle/Resources</directory>
<directory>src/*/Bundle/*Bundle/Resources</directory>
</exclude>
</whitelist>
</filter>

<listeners>
<listener class="CRM\CivixBundle\CivixTestListener">
<arguments/>
Expand Down
5 changes: 2 additions & 3 deletions scripts/make-snapshots.sh
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,8 @@ function build_snapshot() {
cv en "$EXMODULE"
find tests -name '*.php' | xargs civilint
if [ -e 'tests/phpunit' ]; then
## FIXME: phpunit8; but some tests need updating?
phpunit8 --group headless
phpunit8 --group e2e
phpunit9 --group headless
phpunit9 --group e2e
fi
popd
fi
Expand Down
4 changes: 2 additions & 2 deletions scripts/run-tests.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash
set -e

## Run PHPUnit tests. This is a small wrapper for `phpunit8` which does some setup for the E2E enviroment.
## Run PHPUnit tests. This is a small wrapper for `phpunit` which does some setup for the E2E enviroment.

################################################
## Didn't set a workspace? Educated guess...
Expand All @@ -18,4 +18,4 @@ fi
[ ! -d "$CIVIX_WORKSPACE" ] && mkdir -p "$CIVIX_WORKSPACE" || echo ''
(cd "$CIVIX_WORKSPACE" && civibuild restore)

phpunit8 "$@"
phpunit9 "$@"
2 changes: 1 addition & 1 deletion scripts/upgrade-snapshots.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ fi
################################################
export SNAPSHOT_SAVE=1
export SNAPSHOT_FILTER="$1"
phpunit8 tests/e2e/SnapshotUpgradeTest.php --debug
phpunit9 tests/e2e/SnapshotUpgradeTest.php --debug
1 change: 1 addition & 0 deletions shell.nix
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ in
buildkit.pkgs.composer
buildkit.pkgs.pogo
buildkit.pkgs.phpunit8
buildkit.pkgs.phpunit9

pkgs.bash-completion
];
Expand Down
6 changes: 6 additions & 0 deletions src/CRM/CivixBundle/Builder/Collection.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
* A collection of builders
*/
class Collection implements Builder {

/**
* @var array
*/
public $builders;

public function __construct($builders = []) {
$this->builders = $builders;
}
Expand Down
23 changes: 20 additions & 3 deletions src/CRM/CivixBundle/Builder/License.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
namespace CRM\CivixBundle\Builder;

use CRM\CivixBundle\Utils\Files;
use SimpleXMLElement;
use DOMDocument;
use CRM\CivixBundle\Builder;
use Symfony\Component\Console\Output\OutputInterface;

Expand All @@ -15,7 +13,26 @@ class License implements Builder {
protected $name;

/**
* @param $overwrite scalar; TRUE (always overwrite), FALSE (preserve with error), 'ignore' (preserve quietly)
* @var \LicenseData\License
*/
protected $license;

/**
* @var string
*/
protected $path;

/**
* @var bool|string
* TRUE (always overwrite), FALSE (preserve with error), 'ignore' (preserve quietly)
*/
protected $overwrite;

/**
* @param \LicenseData\License $license
* @param string $path
* @param bool|string $overwrite
* TRUE (always overwrite), FALSE (preserve with error), 'ignore' (preserve quietly)
*/
public function __construct(\LicenseData\License $license, $path, $overwrite) {
$this->license = $license;
Expand Down
5 changes: 5 additions & 0 deletions src/CRM/CivixBundle/Builder/Module.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@

class Module implements Builder {

/**
* @var \Symfony\Component\Templating\EngineInterface
*/
public $templateEngine;

public function __construct($templateEngine) {
$this->templateEngine = $templateEngine;
}
Expand Down
11 changes: 7 additions & 4 deletions src/CRM/CivixBundle/Builder/PhpUnitXML.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
namespace CRM\CivixBundle\Builder;

use SimpleXMLElement;
use CRM\CivixBundle\Builder\XML;

/**
* Build/update info.xml
Expand All @@ -22,13 +21,17 @@ public function init(&$ctx) {
$xml->addAttribute('stopOnFailure', 'false');
$xml->addAttribute('cacheResult', 'false');
$xml->addAttribute('bootstrap', 'tests/phpunit/bootstrap.php');

$xml->registerXPathNamespace('xsi', 'http://www.w3.org/2001/XMLSchema-instance');
$xml->addAttribute('xsi:noNamespaceSchemaLocation', 'https://schema.phpunit.de/9.3/phpunit.xsd', 'http://www.w3.org/2001/XMLSchema-instance');

$this->set($xml);

$this->addTestSuite('My Test Suite', ['./tests/phpunit']);

$this->get()
->addChild('filter')
->addChild('whitelist')
->addChild('coverage')
->addChild('include')
->addChild('directory', './')
->addAttribute('suffix', '.php');

Expand All @@ -42,7 +45,7 @@ public function init(&$ctx) {
* @param array $dirs
*/
public function addTestSuite($name, $dirs) {
$testsuites = $this->get()->addChild('testsuites'); // FIXME: find/load
$testsuites = $this->get()->addChild('testsuites'); /* FIXME: find/load */
$testsuite = $testsuites->addChild('testsuite');
$testsuite->addAttribute('name', $name);
foreach ($dirs as $dir) {
Expand Down
4 changes: 2 additions & 2 deletions src/CRM/CivixBundle/Resources/views/Code/test-e2e.php.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ public function tearDown(): void {
*/
public function testWellFormedVersion(): void {
$this->assertNotEmpty(E::SHORT_NAME);
$this->assertRegExp('/^([0-9\.]|alpha|beta)*$/', \CRM_Utils_System::version());
$this->assertMatchesRegularExpression('/^([0-9\.]|alpha|beta)*$/', \CRM_Utils_System::version());
}

/**
* Example: Test that we're using a real CMS (Drupal, WordPress, etc).
*/
public function testWellFormedUF(): void {
$this->assertRegExp('/^(Drupal|Backdrop|WordPress|Joomla)/', CIVICRM_UF);
$this->assertMatchesRegularExpression('/^(Drupal|Backdrop|WordPress|Joomla)/', CIVICRM_UF);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function tearDown():void {
*/
public function testWellFormedVersion():void {
$this->assertNotEmpty(E::SHORT_NAME);
$this->assertRegExp('/^([0-9\.]|alpha|beta)*$/', \CRM_Utils_System::version());
$this->assertMatchesRegularExpression('/^([0-9\.]|alpha|beta)*$/', \CRM_Utils_System::version());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function tearDown(): void {
*/
public function testWellFormedVersion(): void {
$this->assertNotEmpty(E::SHORT_NAME);
$this->assertRegExp('/^([0-9\.]|alpha|beta)*$/', \CRM_Utils_System::version());
$this->assertMatchesRegularExpression('/^([0-9\.]|alpha|beta)*$/', \CRM_Utils_System::version());
}

/**
Expand Down
4 changes: 1 addition & 3 deletions tests/e2e/CRMNamingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
namespace E2E;

use CRM\CivixBundle\Builder\Info;
use Symfony\Component\Console\Input\ArgvInput;
use Symfony\Component\Console\Output\NullOutput;

class CRMNamingTest extends \PHPUnit\Framework\TestCase {

Expand All @@ -29,7 +27,7 @@ public function setUp(): void {
'civix_crmnaming.civix.php' => 1,
]);

\Civix::ioStack()->push(new ArgvInput(), new NullOutput());
\Civix::ioStack()->push(...$this->createInputOutput());
$this->upgrader = \Civix::generator(static::getExtPath());
$this->upgrader->updateInfo(function(Info $info) {
// FIXME: Allow "_" instead of "/"
Expand Down
4 changes: 1 addition & 3 deletions tests/e2e/CiviNamingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
namespace E2E;

use CRM\CivixBundle\Builder\Info;
use Symfony\Component\Console\Input\ArgvInput;
use Symfony\Component\Console\Output\NullOutput;

class CiviNamingTest extends \PHPUnit\Framework\TestCase {

Expand All @@ -29,7 +27,7 @@ public function setUp(): void {
'civix_civinaming.civix.php' => 1,
]);

\Civix::ioStack()->push(new ArgvInput(), new NullOutput());
\Civix::ioStack()->push(...$this->createInputOutput());
$this->upgrader = \Civix::generator(static::getExtPath());
$this->upgrader->updateInfo(function(Info $info) {
// FIXME: Allow "\" instead of "/"
Expand Down
13 changes: 13 additions & 0 deletions tests/e2e/CivixProjectTestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
use CRM\CivixBundle\Utils\Files;
use CRM\CivixBundle\Utils\Path;
use ProcessHelper\ProcessHelper as PH;
use Symfony\Component\Console\Input\ArgvInput;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Output\NullOutput;
use Symfony\Component\Console\Output\StreamOutput;
use Symfony\Component\Console\Tester\CommandTester;

Expand Down Expand Up @@ -351,4 +353,15 @@ protected function assertStringSequence(array $expectSubstrings, string $actualT
$this->assertTrue($lastPos > 0, 'Should have found multiple lines.');
}

/**
* @return array
* [0 => InputInterface, 1 => OutputInterface]
*/
protected function createInputOutput(array $argv = NULL): array {
$input = new ArgvInput($argv);
$input->setInteractive(FALSE);
$output = new NullOutput();
return [$input, $output];
}

}
2 changes: 1 addition & 1 deletion tests/e2e/IdempotentUpgradeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function testBasicUpgrade(): void {
'General upgrade',
];
$this->assertStringSequence($expectLines, $result);
$this->assertNotRegExp(';Upgrade v([\d\.]+) => v([\d\.]+);', $result);
$this->assertDoesNotMatchRegularExpression(';Upgrade v([\d\.]+) => v([\d\.]+);', $result);

// Compare before+after
$end = $this->getExtSnapshot();
Expand Down
8 changes: 4 additions & 4 deletions tests/e2e/MixinMgmtTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ public function testAddPageFor530(): void {
]);

$content = file_get_contents('civix_mixinrec.civix.php');
$this->assertRegExp('|function _civix_mixinrec_civix_mixin_polyfill\(\) \{|', $content);
$this->assertRegExp('|_civix_mixinrec_civix_mixin_polyfill\(\);|', $content);
$this->assertMatchesRegularExpression('|function _civix_mixinrec_civix_mixin_polyfill\(\) \{|', $content);
$this->assertMatchesRegularExpression('|_civix_mixinrec_civix_mixin_polyfill\(\);|', $content);
}

public function testAddPageFor545(): void {
Expand All @@ -163,8 +163,8 @@ public function testAddPageFor545(): void {
]);

$content = file_get_contents('civix_mixinrec.civix.php');
$this->assertNotRegExp('|function _civix_mixinrec_civix_mixin_polyfill\(\) \{|', $content);
$this->assertNotRegExp('|_civix_mixinrec_civix_mixin_polyfill\(\);|', $content);
$this->assertDoesNotMatchRegularExpression('|function _civix_mixinrec_civix_mixin_polyfill\(\) \{|', $content);
$this->assertDoesNotMatchRegularExpression('|_civix_mixinrec_civix_mixin_polyfill\(\);|', $content);
}

}
2 changes: 1 addition & 1 deletion tests/e2e/SnapshotUpgradeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public function checkSnapshot_qf(): void {
$this->assertTrue((bool) preg_match('/^found/', $classExists->getOutput()), 'Class should be loadable/parsable.');

$httpGet = PH::runOk('cv en authx && cv http -LU admin civicrm/my-page');
$this->assertRegExp(';The current time is;', $httpGet->getOutput());
$this->assertMatchesRegularExpression(';The current time is;', $httpGet->getOutput());
}

public function checkSnapshot_svc(): void {
Expand Down