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

Correct use of patchwork.json to redefine internal functions? #97

Closed
Idealien opened this issue Mar 26, 2021 · 4 comments
Closed

Correct use of patchwork.json to redefine internal functions? #97

Idealien opened this issue Mar 26, 2021 · 4 comments

Comments

@Idealien
Copy link
Contributor

Idealien commented Mar 26, 2021

The PHP Setup docs indicate indicate the known limitation that, "Patchwork will fail on every attempt to redefine an internal function that is missing from the redefinable-internals array of your patchwork.json"

It is unclear if this limitation statement means that BrainMonkey cannot override internals or that additional configuration related to use of a patchwork.json file is required in order for that to occur? If it is the latter, could additional docs be updated to specify more detail on how to override?

Example Scenario

In a local dev environment where ldap_bind is not installed in the environment setting either of these when/stubs for it are successful

//For simplistic cases
Functions\when( 'ldap_bind' )->justReturn( true );

//For data-specific cases
Functions\stubs(
[
	'ldap_bind' => function( $ldap, $username, $password ) {
		switch ( $username ) {
			case 'username@domain':
				return false;
			case 'domain\\username':
				return true;
			default:
				return false;
		}
	},
]);

When running them via a CI/CD pipeline where the ldap modules are installed the tests fail with note:

17:30:21  Patchwork\Exceptions\NotUserDefined: Please include {"redefinable-internals": ["ldap_bind"]} in your patchwork.json.
17:30:21  ldap_connect(): Could not create session handle: Bad parameter to an ldap routine
17:30:21  /tmp/workspace/ype-pwp-ui_feature_code-coverage/public/wp-c/plugins/ldap-login/class-ldap-login.php:126
17:30:21  /tmp/workspace/ype-pwp-ui_feature_code-coverage/public/wp-c/plugins/ldap-login/tests/ldap-Test.php:95

Assertion Test: (Line 95)
self::assertEquals( false, $LDAPLogin->ldap_auth( 'username', 'password', 'ol' ) );

Plugin Code: (Line 126)
$is_bound = ldap_bind( $this->ldap, $bind_user, $password );

Troubleshooting Attempted

Patchwork.json
Either adding it within the plugin directory (same level as phpunit.xml) or the plugin/tests directory (same level as bootstrap.php / MonkeyClass.php and actual test.php files)

{
    "redefinable-internals": [
        "ldap_bind",
        "ldap_connect",
        "ldap_search"
    ]
}

The error about redefine internals went away in CI/CD test, but the same 'Could not create session handle' error remained which gives impression it is not patching the internal functions. When running in local (where ldap_bind does not exist) new blocking error, "Error in bootstrap script: ReflectionException: Function ldap_bind() does not exist"

load patchwork manually in the PHPUnit bootstrap file
Based on an old issue mocking internal functions
No perceived impact

Other files (in case)

Composer.json

{
    "require": {
        "vlucas/phpdotenv": "^3.2",
        "oscarotero/env": "^1.1"
    },
    "require-dev": {
        "brain/monkey": "2.*",
        "phpunit/phpunit": "^9.4"
    }
}

PHPUnit execution
./vendor/phpunit/phpunit/phpunit -c ./public/wp-c/plugins/ldap-login/phpunit.xml --coverage-html ../../coverage-reports/

@XedinUnknown
Copy link

I found this thread because I couldn't find where to put patchwork.json, or what its schema is - even on Patchwork website. Would be helpful if the docs included this. Everything worked for me after I added it, though.

@tfrommen
Copy link
Contributor

@XedinUnknown it's (somewhat hidden) included in the first example on the official Patchwork website:

image

Since documenting the structure and location can be done in just a single sentence, would be good to add this to the Brain Monkey documentation, yeah. But in general, I don't feel this needs to be here.

@XedinUnknown
Copy link

Thanks, @tfrommen! Yes, partly the schema is documented there. And perhaps I should raise an issue in that project. Maybe this will shed some light on the possible locations too.

@gmazzap
Copy link
Collaborator

gmazzap commented Dec 12, 2022

I'm closing this issue because there's nothing to do here. Improvements to the documentation of Patchwork configuration is something for Patchwork repo.

@gmazzap gmazzap closed this as completed Dec 12, 2022
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

4 participants