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

Upgrade bundled PHPUnit #87

Closed
cmb69 opened this issue Nov 15, 2019 · 6 comments
Closed

Upgrade bundled PHPUnit #87

cmb69 opened this issue Nov 15, 2019 · 6 comments

Comments

@cmb69
Copy link
Contributor

cmb69 commented Nov 15, 2019

The files in cache/util/PEAR/pear/PHPUnit are currently PHPUnit 3.6.11, DBUnit 1.2.0 and phpunit-mock-objects; so all pretty old versions, and the latter two projects are even abandoned, whereby the MockObjects have been bundled with PHPUnit in the meantime.

However, the bundled PHPUnit has been patched:

--- b/cache/util/PEAR/pear/PHPUnit/Framework/TestCase.php
+++ a/cache/util/PEAR/pear/PHPUnit/Framework/TestCase.php
@@ -484,6 +484,21 @@
         $this->expectedExceptionCode    = $exceptionCode;
         $this->expectedExceptionTrace   = debug_backtrace();
     }
+	
+	public function pftt_step1() {
+		$this->setUpBeforeClass();
+		$this->setExpectedExceptionFromAnnotation();
+        $this->setUp();
+        $this->checkRequirements();
+        $this->assertPreConditions();
+	}
+	public function pftt_step2() {
+		$this->verifyMockObjects();
+        $this->assertPostConditions();
+	}
+	public function pftt_step3() {
+        $this->tearDown();
+	}
 
     /**
      * @since  Method available since Release 3.4.0
@@ -766,7 +781,6 @@
     public function runBare()
     {
         $this->numAssertions = 0;
-
         // Backup the $GLOBALS array and static attributes.
         if ($this->runTestInSeparateProcess !== TRUE &&
             $this->inIsolation !== TRUE) {
@@ -786,17 +800,15 @@
         }
 
         // Start output buffering.
+        // TODO ob_start();
-        ob_start();
         $this->outputBufferingActive = TRUE;
 
         // Clean up stat cache.
         clearstatcache();
-
         try {
             if ($this->inIsolation) {
                 $this->setUpBeforeClass();
             }
-
             $this->setExpectedExceptionFromAnnotation();
             $this->setUp();
             $this->checkRequirements();
--- b/cache/util/PEAR/pear/PHPUnit/Util/PHP.php
+++ a/cache/util/PEAR/pear/PHPUnit/Util/PHP.php
@@ -115,7 +115,9 @@
                     PHP_BINDIR . '/php',
                     PHP_BINDIR . '/php-cli.exe',
                     PHP_BINDIR . '/php.exe',
+					/* -- begin PFTT -- */
+                    'C:\php-sdk\php-5.4-nts-windows-vc9-x86-r9fe8c58\php.exe',
+					/* -- end PFTT -- */
-                    '@php_bin@',
                 );
                 foreach ($possibleBinaryLocations as $binary) {
                     if (is_readable($binary)) {

Also cache/util/PEAR/phpunit(.bat) have been patched as well.

So we have to figure out the details of upgrading and possibly unbundling:

  • Are these patches necessary? How to do without them?
  • Is DBUnit actually used?
  • Which further adaptions would be needed to upgrade to some supported PHPUnit version?
  • Do we have to stick with a slightly older PHPUnit version, to be able to test the PHP 5.6 and 7.0 backports?
@lavturo
Copy link
Contributor

lavturo commented Nov 20, 2019

To answer a few of these questions:

For the patches, it is hard to say if they are needed as of now since app testing is currently not working. We could keep these as reference if we were to update PHPUnit.

DBUnit is only a part of the PEAR directory. It is part of some .php files. Otherwise, it is not referenced in pftt itself. However, the PEAR directory is referenced once. This one reference is for PFTT to find the autoload.php for PHPUnit (located at pftt2/cache/util/PEAR/pear/PHPUnit). Otherwise, it will give an error about not being able to find said file.

I believe, we would only have to update the test.php file that is generated by PFTT, if we wanted to move or update PHPUnit. However, I am unsure of how test.php used PHPUnit. So I would have to look through that file before commenting on what else needs to be updated or adapted.

If we want to be able to do app testing for PHP 5.6 and 7.0, then we would only be able to upgrade to PHPUnit 5.

@cmb69
Copy link
Contributor Author

cmb69 commented Dec 4, 2019

@lavturo, it's actually that test.php that worries me most. It seems that this is a variant of an instantiated TestCaseMethod.tpl.dist, and as such may be highly dependent on the PHPUnit version (i.e. would likely have to be adapted for every upgrade). It might be worthwhile to have a look at the officially supported customizations (see respective PHPUnit 5 docs); perhaps these already offer what we need?

@lavturo
Copy link
Contributor

lavturo commented Dec 5, 2019

From what I have found, test.php is generated from PhpUnitTemplate.goovy. The first part of it seems similar to the file you linked.

Any change to that file gives the same error: "Class 'WP_UnitTestCase' not found". This error occurs due to the bootstrap file. This makes me wonder if the current wordpress-tests I am using has its own problems or if it is something else all together.

The officially supported customizations seem to be helpful. We wouldn't need to change any of the actual tests themselves though, do we?

@cmb69
Copy link
Contributor Author

cmb69 commented Dec 5, 2019

I ran the wordpress-tests a few weeks ago standalone (i.e. without PFTT), and besides some test failures, that worked fine. I guess that PFTT only was run with some old version of these tests (most likely even before they have been merged into wordpress-develop). Also, the hard-coded paths in PHPUnitTemplate.groovy look fishy; there's certainly no need to include symfony for running WP tests. Possibly, these have been adjusted manually for each app test scenario, and only this "snapshot" has been committed? (see also a few lines below; there are several include paths for different app scenarios; WordPress doesn't seem to be one of them, though)

The PHPUnit customizations should indeed only affect the test runner, so no modifications to the actual tests should be necessary (and we certainly want to avoid anyway).

@lavturo
Copy link
Contributor

lavturo commented Dec 5, 2019

There is a call made in AbstractPhpUnitTestCaseRunner.java that gets the info that is stored in wordpress.groovy.

As for the include path, it is updated and passed from PhpUnitDist.java. It grabs every directory in the specified source test pack directory (which is defined from wordpress.groovy).

I believe the hard-coded paths were left there so that they know what directories are needed if they were to try and run a test for one of those specific app scenarios. However, they might be outdated if the test source packs for those app scenarios have been updated with different directories.

@cmb69
Copy link
Contributor Author

cmb69 commented Jan 17, 2020

Since bf0a98e updated to PHPUnit 5, this ticket can be closed.

@cmb69 cmb69 closed this as completed Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants