10up / wpacceptance

ARCHIVED: A team scalable solution for reliable WordPress acceptance testing.
https://wpacceptance.readthedocs.io/
MIT License
148 stars 15 forks source link

TestCase::last_modifying_query can be empty during tearDown() #19

Closed felipeelia closed 4 years ago

felipeelia commented 4 years ago

After having it working fine for a while, all of a sudden it started displaying the following error:

Trying to access array offset on value of type null vendor/10up/wpacceptance/src/classes/PHPUnit/TestCase.php:67

Steps to Reproduce

I'm not 100% sure about how to reproduce it but it happened to me with a very simple test (I was just activating a plugin). I got it happening locally and on a GitHub Action.

Environment information

Suggested fix There is a big chance I'm ignoring something here but it seems we should change this if (WPAcceptance\PHPUnit\TestCase::tearDown()) from

if ( ! empty( $new_last_modifying_query ) && $new_last_modifying_query['event_time'] !== $this->last_modifying_query['event_time'] ) {

to

if ( ! empty( $this->last_modifying_query ) && ! empty( $new_last_modifying_query ) && $new_last_modifying_query['event_time'] !== $this->last_modifying_query['event_time'] ) {

Although $this->last_modifying_query is initially set as an empty array, it is set as $this->last_modifying_query = $this->getLastModifyingQuery(); during setUp(). If any query run, this will be null, triggering the error during tearDown().

Please let me know if this all makes sense and I can open a PR if it helps. Thanks in advance!

imgerson commented 4 years ago

@felipeelia I think I encountered the same error running tests on php 7.4. Reverting to 7.3 solved it for me. Just my 50 cents on this :)

jeffpaul commented 4 years ago

@felipeelia per @imgerson's note, could you try this on PHP 7.3 and see if the issue continues/resolves?

felipeelia commented 4 years ago

Sure @jeffpaul, I'll try to give a look at this during the weekend. Although I could not spot why, it does seem to be related to the PHP version. GitHub Actions just started to use PHP 7.4 these days and then the error started showing. In the meantime, @imgerson, can you please test if the solution I've suggested works for you too? Thank you both!

imgerson commented 4 years ago

Sure @felipeelia, I'll give it a try after tonight and come back with a comment 😉

imgerson commented 4 years ago

Sorry for being late to the party @felipeelia.

I installed PHP 7.4.2 and ran tests with the following:

// FILE: wpacceptance.json
{
    "name": "name",
    "tests": [
        "tests\/*.php"
    ],
    "environment_instructions": [
        [
            "install wordpress where url is http://domain-tests.test",
            "add site where url is http://domain-tests.test and title is Test!",
            "install theme where name is twentynineteen"
        ]
    ],
    "project_path": "%WP_ROOT%/wp-content"
}

And the following standard tests:

FILE: tests/standard.php
<?php
/**
 * Test standard WP functionality
 *
 */

/**
 * PHPUnit test class
 */
class StandardTest extends \WPAcceptance\PHPUnit\TestCase {

    /**
     * @testdox I see required HTML tags on front end.
     */
    public function testRequiredHTMLTagsOnFrontEnd() {
        parent::_testRequiredHTMLTags();
    }

    /**
     * @testdox I can log in.
     */
    public function testLogin() {
        parent::_testLogin();
    }

    /**
     * @testdox I see the admin bar
     */
    public function testAdminBarOnFront() {
        parent::_testAdminBarOnFront();
    }

    /**
     * @testdox I can save my profile
     */
    public function testProfileSave() {
        parent::_testProfileSave();
    }

    /**
     * @testdox I can install a plugin
     */
    public function testInstallPlugin() {
        parent::_testInstallPlugin();
    }

    /**
     * @testdox I can change the site title
     */
    public function testChangeSiteTitle() {
        parent::_testChangeSiteTitle();
    }
}

And they actually run fine, I can't reproduce that error anymore :/

felipeelia commented 4 years ago

Hey @imgerson, thanks for the reply!

In the meantime, I've opened the #20 PR suggesting a change that hopefully will fix the problem for everybody. I'm not that familiar with the ins and outs of WP Acceptance, so I'll try to find some time during this week to use your setup and see if it works on my side as well. I can see a few differences between our tests and wpacceptance.json file, probably it is worth investigating.