search

woocommerce / action-scheduler-custom-tables

This plugin is no longer needed. The custom table code is now part of Action Scheduler version 3.0 and newer. If you're using the latest Action Scheduler, you have the most performant schema available.
https://actionscheduler.org
15 stars 3 forks source link

Update for Prospress/action-scheduler#113 #19

Closed thenbrent closed 6 years ago

thenbrent commented 6 years ago

This adds a bunch of changes to make the custom tables compatible with https://github.com/Prospress/action-scheduler/pull/113. See commit messages for details.

I need to do a final review on these now that https://github.com/Prospress/action-scheduler/pull/113 is merged - most of the commits were made a while ago, but I wanted to get a PR up now so the changes are visible (and to avoid something like https://github.com/Prospress/action-scheduler-custom-tables/pull/17 happening again).

This PR still includes https://github.com/Prospress/action-scheduler-custom-tables/commit/80ade0a23873ce41c3c145c667b2e8a3a8fb4561, which is equivalent, but not as good as, https://github.com/Prospress/action-scheduler-custom-tables/pull/17 (so the patches in that PR should be used instead). I'll either revert or remove SHA: 80ade0a before proposing this for final review.

thenbrent commented 6 years ago

Thanks for the review @JPry. I've fixed up all of those issues!

JPry commented 6 years ago

@thenbrent I've made some updates to the tests, but I'm still seeing some failures I can't explain. I have the master branch of Action Scheduler running alongside this particular branch. Would you mind taking a look at the failing tests, as well as the changes I made? Here's the output I'm seeing:

phpunit@5.7 -c tests/phpunit.xml.dist                  
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 5.7.27 by Sebastian Bergmann and contributors.

...............................................................  63 / 188 ( 33%)
.....................FF........................FF.............. 126 / 188 ( 67%)
..............................................................  188 / 188 (100%)

Time: 3.83 seconds, Memory: 14.00MB

There were 4 failures:

1) Action_Scheduler\Custom_Tables\Migration_Scheduler_Test::test_scheduler_runs_migration
Failed asserting that actual size 10 matches expected size 15.

/Users/jpry/Sites/wordpress-develop/src/wp-content/plugins/action-scheduler-custom-tables/tests/phpunit/migration/Migration_Scheduler_Test.php:74
/Users/jpry/Sites/wordpress-develop/src/wp-content/plugins/action-scheduler-custom-tables/tests/UnitTestCase.php:43

2) Action_Scheduler\Custom_Tables\Migration_Scheduler_Test::test_scheduler_runs_migration
Failed asserting that actual size 10 matches expected size 15.

/Users/jpry/Sites/wordpress-develop/src/wp-content/plugins/action-scheduler-custom-tables/tests/phpunit/migration/Migration_Scheduler_Test.php:74
/Users/jpry/Sites/wordpress-develop/src/wp-content/plugins/action-scheduler-custom-tables/tests/UnitTestCase.php:46

3) procedural_api_Test::test_unschedule
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'59b0c42b95d8111e1786c61364fdc305'
+''

/Users/jpry/Sites/wordpress-develop/src/wp-content/plugins/action-scheduler/tests/phpunit/procedural_api/procedural_api_Test.php:67
/Users/jpry/Sites/wordpress-develop/src/wp-content/plugins/action-scheduler/tests/phpunit/deprecated/ActionScheduler_UnitTestCase.php:35

4) procedural_api_Test::test_unschedule
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'7ab22e9d773cddfc1b5f11ec2027f5e0'
+''

/Users/jpry/Sites/wordpress-develop/src/wp-content/plugins/action-scheduler/tests/phpunit/procedural_api/procedural_api_Test.php:67
/Users/jpry/Sites/wordpress-develop/src/wp-content/plugins/action-scheduler/tests/phpunit/deprecated/ActionScheduler_UnitTestCase.php:38

FAILURES!
Tests: 564, Assertions: 518, Failures: 4.
JPry commented 6 years ago

@thenbrent I finally found the issue that was causing the tests to fail, and everything is passing now. I'm marking this approved, and I'll leave it to you to merge since I added more, starting with https://github.com/Prospress/action-scheduler-custom-tables/pull/19/commits/bf4ea66495b6e558dd3748167cdc44655c3c23e5.

JPry commented 6 years ago

I went back to work on #17 after this, and it looks like any new work is going to depend on this PR. So as not to hold anything up, I'm going to go ahead and merge this. @thenbrent feel free to review the commits I added, and if necessary we can create a new PR to change anything. I'll leave the branch available for now instead of deleting it.

thenbrent commented 6 years ago

feel free to review the commits I added

Those all LGTM, thanks @JPry 🙇