SilverStripe 4.0 Compatability #125

brettt89 · 2017-04-21T14:59:18Z

Update module to be compatible with SilverStripe 4.0

sminnee

One small question - appears to be a functional change rather than upgrade and looks like it would gobble some errors.

sminnee · 2017-04-22T01:05:40Z

src/Services/QueuedJobService.php

@@ -680,7 +681,7 @@ public function runJob($jobId)
                }
            } catch (Exception $e) {
                // okay, we'll just catch this exception for now
-                $this->getLogger()->error($e->getMessage());
+                $this->getLogger()->debug($e->getMessage());


Why have you changed the level of these messages?

I had done so after reading https://docs.silverstripe.org/en/4/developer_guides/debugging/error_handling/

However I misread it as "debug and warn" should be through logger and "user_error" for errors.

…ory_limit of jobs

brettt89 · 2017-04-22T01:46:50Z

Have been running through some tests manually and have found a couple of bugs that don't look to be covered by tests. Working through those now.

brettt89 · 2017-04-22T01:47:16Z

Date fields weren't updated. Updating these and adding tests for checking correct dates.

…orm date/time submission

edlinklater · 2017-04-22T02:46:28Z

Fixes #126

brettt89 · 2017-04-23T13:48:41Z

Fixes #122

robbieaverill · 2017-04-23T21:12:51Z

tests/QueuedJobsTest/TestQJService.php

+use SilverStripe\QueuedJobs\Services\QueuedJobService;
+
+// stub class to be able to call init from an external context
+class TestQJService extends QueuedJobService


These stubs should probably implement SilverStripe\Dev\TestOnly (I know they didn't before)

If I add "TestOnly" to these objects, the tests fail as SapphireTest ignores buildings the database for these objects.

https://github.com/silverstripe/silverstripe-framework/blob/master/src/Dev/SapphireTest.php#L1173

robbieaverill · 2017-04-23T21:13:21Z

tests/ScheduledExecutionTest.php

@@ -118,9 +125,10 @@ public function testScheduledExecutionInterval()
    }
 }

-
-class TestScheduledDataObject extends DataObject implements TestOnly
+class TestScheduledDataObject extends DataObject


Add test only back

If I add "TestOnly" to these objects, the tests fail as SapphireTest ignores buildings the database for these objects.

https://github.com/silverstripe/silverstripe-framework/blob/master/src/Dev/SapphireTest.php#L1173

And the DB table is required due to the write occuring in the test
https://github.com/brettt89/silverstripe-queuedjobs/blob/compat-4.0/tests/ScheduledExecutionTest.php#L33

Sounds like a bug to me!

sminnee · 2017-04-24T00:43:32Z

tests/ScheduledExecutionTest.php

+     */
+    protected $usesDatabase = true;
+
+    /**
     * {@inheritDoc}
     * @var array
     */
    protected $extraDataObjects = array(


this should be protected static $extra_dataobjects

…ataObjects to extra_dataobjects static variable

brettt89 · 2017-04-24T01:21:32Z

@sminnee @robbieaverill Have updated tests to correctly implement TestOnly.

Warnings in tests are due to expected errors thrown by QueuedJobs.

sminnee · 2017-04-24T01:24:59Z

OK those warnings block merge. We'll need to think of a better way of handling these.

Perhaps the tests need to mock in a logger than can pick up these issues and validate that they have appeared?

robbieaverill · 2017-04-24T01:38:03Z

Yeah, you can push a NullHandler from monolog to the logger from the setUp method so it won't default to logging these to the terminal.

We could also push mocked handlers instead with expectations on how the methods getting called, but that's possibly not within the scope of the specific unit tests that exist.

brettt89 · 2017-04-24T02:18:12Z

@sminnee @robbieaverill Have pushed a NullHandler onto the ErrorHandler using Injector. PHPUnit is not reporting them as "Warnings", however the messages are still appearing in the output. Is that going to be an issue?

sminnee · 2017-04-24T03:12:55Z

It makes it look broken, which inhibits all future dev, so yes I think we want to sort it out.

sminnee · 2017-04-24T03:13:34Z

Are you sure that the messages are, in fact, benign?

robbieaverill · 2017-04-24T03:17:53Z

src/QJUtils.php

@@ -5,6 +5,7 @@
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\Injector;
 use SilverStripe\ORM\DB;
+use Monolog\Logger;


Is this used in the class?

robbieaverill · 2017-04-24T03:18:11Z

.travis.yml

@@ -5,7 +5,6 @@ sudo: false
 language: php

 php:
-  - 5.5


May as well add 7.1 while you're there

Added in commit 2af7805

brettt89 · 2017-04-24T04:16:27Z

@sminnee Have updated QueuedJobsTest to catch errors and check that expected errors exist after specific tests. Have confirmed that errors are expected as it is testing how the system reacts after failure.

@robbieaverill Have removed reference to Monologger/Logger and added tests for 7.1

…d classes

nyeholt · 2017-04-27T01:52:33Z

@sminnee did Brett's last update resolve the issue you had?

sminnee · 2017-04-27T23:57:32Z

Yeah I'm happy with this now — no gunk in the test output.

I'd note that we probably don't need so many build configs and that the following should be sufficient:

PHP 5.6 + MYSQL
PHP 7.0 + PGSQL
PHP 7.1 + MYSQL

There's a finite number of concurrent builds across all github.com/silverstripe-australia runs so it's good to keep your job counts in check. ;-)

robbieaverill · 2017-04-30T22:54:45Z

Assuming this PR fixes #122 (it's noted above as doing so, and the builds pass so I assume that's true) then it replaces #123 too (also replaced by #124). I've closed #123.

brettt89 added 9 commits April 22, 2017 02:56

SS 4.0 - Replace DoormanProcessManager in DoormanRunner

79580e7

SS 4.0 - Fix Cleanup Job + Tests

18f6661

SS 4.0 - Fix QueuedJobsAdmin + Tests

e2c375b

SS 4.0 - Fix QueuedJobService, QJUtils and QueuedJobsTests

949f61c

SS 4.0 - Fix ScheduledExecutionTests

8526590

SS 4.0 - Add phpunit.xml.dist

b6dbb7d

SS 4.0 - Remove PHP 5.5 tests from .travis.yml

8e74bc8

SS 4.0 - Add silverstripe/admin to composer requirements

0ff1285

SS 4.0 - Revert createMock back to getMock for PHPUnit 3.x compatibility

45b1486

sminnee suggested changes Apr 22, 2017

View reviewed changes

flamerohr approved these changes Apr 22, 2017

View reviewed changes

brettt89 added 2 commits April 22, 2017 13:21

SS 4.0 - Revert Logging methods to error, add note for increasing mem…

e85495a

…ory_limit of jobs

SS 4.0 - Update README.md to reflect changes

f0c9a74

SS 4.0 - Replace Config::inst()->update() with modfiy()->set(), fix f…

42cc8fe

…orm date/time submission

robbieaverill reviewed Apr 23, 2017

View reviewed changes

sminnee reviewed Apr 24, 2017

View reviewed changes

brettt89 added 2 commits April 24, 2017 12:53

SS 4.0 - Add 'TestOnly' implementation to test objects, update extraD…

2a0e505

…ataObjects to extra_dataobjects static variable

SS 4.0 - Fix namespace of TestQueuedJob

a1c52e7

SS 4.0 - Add NullHandler to ErrorHandler for QueuedJobsTest

07a774a

brettt89 closed this Apr 24, 2017

brettt89 reopened this Apr 24, 2017

robbieaverill reviewed Apr 24, 2017

View reviewed changes

brettt89 added 5 commits April 24, 2017 15:57

ADD: Different log message in lang to match function

31ce28e

SS 4.0 - Add PHP 7.1 to travis tests

2af7805

SS 4.0 - Add support for PHPUnit 3 and PHPUnit 5+

deb234d

SS 4.0 - Add error capture and checking on stalled jobs tests

f031c4c

SS 4.0 - Remove reference to Monologger/Logger in QJUtils

842c432

SS 4.0 - Increase Implimentation character length to handle namespace…

1fdba34

…d classes

sminnee approved these changes Apr 27, 2017

View reviewed changes

robbieaverill mentioned this pull request Apr 30, 2017

Update doorman to ^3.0 #123

Closed

brettt89 mentioned this pull request May 1, 2017

WIP: Compat4/btasker silverstripe/silverstripe-fulltextsearch#141

Merged

nyeholt merged commit 82d0cd7 into symbiote:master May 2, 2017

robbieaverill mentioned this pull request May 5, 2017

Update asyncphp/doorman to ^3.0 #122

Closed

robbieaverill mentioned this pull request Jun 19, 2017

Class SilverStripe\\QueuedJobs\\Jobs\\DoormanProcessManager may not inherit from final class (AsyncPHP\\Doorman\\Manager\\ProcessManager) #126

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SilverStripe 4.0 Compatability #125

SilverStripe 4.0 Compatability #125

brettt89 commented Apr 21, 2017

sminnee left a comment

sminnee Apr 22, 2017

brettt89 Apr 22, 2017

brettt89 commented Apr 22, 2017

brettt89 commented Apr 22, 2017

edlinklater commented Apr 22, 2017

brettt89 commented Apr 23, 2017

robbieaverill Apr 23, 2017

brettt89 Apr 24, 2017

robbieaverill Apr 23, 2017

brettt89 Apr 24, 2017

robbieaverill Apr 24, 2017

sminnee Apr 24, 2017

brettt89 commented Apr 24, 2017

sminnee commented Apr 24, 2017

robbieaverill commented Apr 24, 2017 •

edited

brettt89 commented Apr 24, 2017 •

edited

sminnee commented Apr 24, 2017

sminnee commented Apr 24, 2017

robbieaverill Apr 24, 2017

robbieaverill Apr 24, 2017

brettt89 Apr 24, 2017

brettt89 commented Apr 24, 2017

nyeholt commented Apr 27, 2017

sminnee commented Apr 27, 2017

robbieaverill commented Apr 30, 2017

SilverStripe 4.0 Compatability #125

SilverStripe 4.0 Compatability #125

Conversation

brettt89 commented Apr 21, 2017

sminnee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brettt89 commented Apr 22, 2017

brettt89 commented Apr 22, 2017

edlinklater commented Apr 22, 2017

brettt89 commented Apr 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brettt89 commented Apr 24, 2017

sminnee commented Apr 24, 2017

robbieaverill commented Apr 24, 2017 • edited

brettt89 commented Apr 24, 2017 • edited

sminnee commented Apr 24, 2017

sminnee commented Apr 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brettt89 commented Apr 24, 2017

nyeholt commented Apr 27, 2017

sminnee commented Apr 27, 2017

robbieaverill commented Apr 30, 2017

robbieaverill commented Apr 24, 2017 •

edited

brettt89 commented Apr 24, 2017 •

edited