[RFC|Merged] Modular cron

[igorw]

These two changesets should be merged, and I suggest manually applying yours on top of Andreas' changes. I'll take care of it.

Done: https://github.com/igorw/phpbb3/compare ... ystem-cron (please reset yours)

This is basically for MOD authors, a simple guide telling them how to write a cron task and maybe for administrators how to set up real cron for this. I will take care of this. Consider it assigned to me.

And done: http://wiki.phpbb.com/display/DEV/Modular+cron

[bantu]

Good job. Let's get this integrated soonish.

[Oleg]

I started on the tests:

https://github.com/p/phpbb3/commit/b3c3 ... 03c62fdc36

I found it necessary to set up the autoloader in testing.

[igorw]

Might want to make the autoloader available in all tests, moving that stuff to framework.php. Your indentation is not consistent. And please add the ticket ID to the end of the commit message (see other commits). And the copyright year is wrong in the header, there is also $id$ present.

Looks like a good start.

[bantu]

And please add the ticket ID to the end of the commit message (see other commits).

Most of the commits do not contain a ticket ID, so one has to go through those anyway (better sooner than later). Also, develop should probably have never been merged back into the feature-branch, the feature-branch should have been rebased because that is much cleaner. Maybe you can fix both things at the same time.

[Oleg]

I adjusted the commit:

https://github.com/p/phpbb3/commit/cf70 ... 9d62ada982

In my estimation, rebasing on top of develop should present relatively few conflicts, the schema changes being an obvious point of contention.

The basic tests are all positive, from Igor's earlier comment I am led to believe that a negative test is required - that cron manager should not return a task that is not ready to run.

[igorw]

Might want to make the autoloader available in all tests, moving that stuff to framework.php.

Please look at this.

Most of the commits do not contain a ticket ID, so one has to go through those anyway (better sooner than later). Also, develop should probably have never been merged back into the feature-branch, the feature-branch should have been rebased because that is much cleaner. Maybe you can fix both things at the same time.

Working on this.

The basic tests are all positive, from Igor's earlier comment I am led to believe that a negative test is required - that cron manager should not return a task that is not ready to run.

Yes, that would be good. IMO the current tests make too many assumptions about the environment. You are just testing if the cron manager is returning any tasks in most cases. It would be better to give the cron manager some kind of mock tasks. So we can test different scenarios. What happens when a task is runnable, what when it should run. Then combinations and more than one task. And parametrized tasks. When testing the cron manager, we should not make assumptions about the task implementations (here's where dependency injection comes in handy).

The other thing would be testing the actual task implementations. I believe PHPUnit provides a way to set globals that are reset after every test case (possibly even automatically). That may be useful when testing things like tidy_cache, allowing you to create a "fake" mock cache object that does nothing, and you can just check if it run or not (take a look at class_loader_test, it uses a cache mock).

I hope that gives you some more ideas for testing.

[igorw]

Most of the commits do not contain a ticket ID, so one has to go through those anyway (better sooner than later). Also, develop should probably have never been merged back into the feature-branch, the feature-branch should have been rebased because that is much cleaner. Maybe you can fix both things at the same time.

And done: https://github.com/igorw/phpbb3/compare ... ystem-cron

Please review, then reset your own feature/system-crom.

[bantu]

Most of the commits do not contain a ticket ID, so one has to go through those anyway (better sooner than later). Also, develop should probably have never been merged back into the feature-branch, the feature-branch should have been rebased because that is much cleaner. Maybe you can fix both things at the same time.

And done: https://github.com/igorw/phpbb3/compare ... ystem-cron

Please review, then reset your own feature/system-crom.

Looks good, some commit messages however exceed the 80 characters per line limit.

[igorw]

Fixed.