[RFC|Merged] Modular cron

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Post Reply
User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Sun Dec 19, 2010 10:59 pm

igorw wrote: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)
igorw wrote: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

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Modular cron

Post by bantu » Mon Dec 20, 2010 12:02 am

Good job. Let's get this integrated soonish. :)

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Modular cron

Post by Oleg » Mon Dec 20, 2010 3:50 am

I started on the tests:

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

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

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Mon Dec 20, 2010 7:14 am

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.

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Modular cron

Post by bantu » Mon Dec 20, 2010 10:26 pm

igorw wrote: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
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Modular cron

Post by Oleg » Mon Dec 20, 2010 11:57 pm

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.

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Tue Dec 21, 2010 7:13 am

igorw wrote:Might want to make the autoloader available in all tests, moving that stuff to framework.php.
Please look at this.
bantu wrote: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.
nn- wrote: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.

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Tue Dec 21, 2010 9:31 am

bantu wrote: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.
Last edited by bantu on Tue Dec 21, 2010 12:03 pm, edited 1 time in total.
Reason: quote="igorw" => quote="bantu"

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC|Accepted] Modular cron

Post by bantu » Tue Dec 21, 2010 1:21 pm

igorw wrote:
bantu wrote: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.

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Modular cron

Post by igorw » Tue Dec 21, 2010 4:23 pm

Fixed.

Post Reply