Remove the Disclaimers widget and functions from Largo #1501
Conversation
corrects comments in tests for largo-post-series-links and largo-series-posts removes post metaboxes removes sitewide options
benlk
changed the title
|
Here's the testing workflow for this PR:
|
|
The present problem is that, when the update function AJAX call is initialized, the optionsframework option is set, but when the optionframework update is made, the option is empty: 2018/07/26 19:07:51 [error] 328#0: *2551 FastCGI sent in stderr: "PHP message: 'This isn't the default disclaimer widget setting.' |
|
PHP message: 'This isn't the default disclaimer widget setting.' So it's something in the function that sets Largo's defaults. |
|
PHP message: 'running updates: 'This isn\'t the default disclaimer widget setting.'' |
…ility earlier in largo_perform_update to where the defaults haven't yet been set. I do not know why the thing was not working, but it was not. Maybe this works?
It's not ralistic to expect that people will be using INN/deploy-tools to run tests, especially when the test-runner there assumes a Vagrant box that is unmaintained and no longer used by INN. Therefore, generally follow the instructions in https://make.wordpress.org/cli/handbook/plugin-unit-tests/ you@yours:~/sites/largo/wp-content/themes/largo$ WP_TESTS_DIR=/tmp/wordpress-tests-lib/ bin/install-wp-tests.sh <dbname> <dbuser> <dbpass> <dbhost> latest you@yours:~/sites/largo/wp-content/themes/largo$ WP_TESTS_DIR=/tmp/wordpress-tests-lib/ phpunit --verbose
…ertion that should be tested separately
In the interests of not thrashing travis, I tried to set up tests locally, by following the instructions at https://make.wordpress.org/cli/handbook/plugin-unit-tests/ as documented in 5d99f1a: This works, up to the point where the tests are run:
This is a difference in behavior between the local tests and Travis; Travis does not have this problem. I'm also running the local tests on PHP 7.2 with PHPUnit 6.5.9, and Travis is running them against all current PHP versions except 7.2, as mentioned in #1503. The relevant error is this call to As far as I can tell by putting some debugging code in the bootstrap file, before the function benlk_test( $value ) {
if ( preg_match( '/largo/', $value ) ) {
return true;
}
return false;
}
function benlk_ack() {
$files = get_included_files();
echo(var_export( array_filter( $files, 'benlk_test' ), true));
}They were added just before the final line of bootstrap.php: The result was as follows: array (
534 => '/Users/blk/sites/largo/wp-content/themes/largo/tests/bootstrap.php',
538 => '/Users/blk/sites/largo/wp-content/themes/largo/tests/mock/mock-options-framework.php',
539 => '/Users/blk/sites/largo/wp-content/themes/largo/tests/mock/mock-admin-functions.php',
)There were no other Largo files than those three active at this time. I believe what's blocking me from running tests locally is twofold:
And yet, this works on Travis. |
|
Activating the theme that way causes no detriments to the (failing) Travis tests, and the tests now run on my computer with PHP 7.2. Small progress. |
|
By applying the reflection function technique seen in that StackOverflow link, we can see that the function test_largo_disclaimers_plugin_compatibility_3() {
// https://stackoverflow.com/a/2222404
$reflfunc = new ReflectionFunction( 'of_get_option' );
echo "\n get: " . $reflfunc->getFilename() . ':' . $reflfunc->getStartLine();
$set = new ReflectionFunction( 'of_get_option' );
echo "\n set: " . $set->getFilename() . ':' . $set->getStartLine();
echo "\n";
}So ... this isn't a problem with the optionsframework. Something's wrong with either the test or the function itself. I'm still really, really confused why setting an optionsframework option in one line, then in the function called on the next line, the optionsframework option retrieved by the same mock framework as the setter used is something different entirely, an unset value. For tomorrow: Within a single test, determine whether a setter and getter with no intervening calls results in the getter retrieving what the setter set.I don't want to have to test the optionsframework mock, but maybe that's the place to start tomorrow. |
// test that of_get_option and of_set_option work
function test_largo_disclaimers_plugin_compatibility_3() {
$message = 'test message:' . md5( date( 'N' ) );
$option_key = 'reallyshouldnothavetodothis';
of_set_option( $option_key, $message );
$option = of_get_option( $option_key );
$this->assertEquals( $message, $option, 'wtf' );
}That test passes, so the optionsframework getter/setter mock works. // test that of_get_option and of_set_option work
function test_largo_disclaimers_plugin_compatibility_3() {
$message = 'test message:' . md5( date( 'N' ) );
$option_key = 'reallyshouldnothavetodothis';
update_option( $option_key, $message );
$option = get_option( $option_key );
$this->assertEquals( $message, $option, 'wtf' );
}That test passes, so the WordPress core options stuff works. So something is wrong with the test or with the migration function. Further debugging of the test and migration function is needed, and I don't have time for that today. Next steps:
|
|
Bumping this out of the 0.6 milestone as well |
|
tests may be failing locally because the plugin would already be installed in a local testing env. |
This PR removes the "Disclaimers" functionality from Largo in favor of the plugin https://github.com/INN/disclaimers.
Changes
Things not changed
Closes #1500.