Tests needed

I think the best place to start for writing tests is to add some manual trigger that a test can push to generate a newsletter.

This is probably best in the form of a variable that is passed to represent what is "now" instead of current time()

from this point, newsletter generation should be testable

I've started adding some tests, didnt need to go as far as the above comment tho, just set the date to a day previous.

this patch does not yet smartly set the date, it just choose the start of the month (!)

the bug problem is i'm seeing this when i call simplenews_scheduler_cron();, ideas?

Undefined property: stdClass::$simplenews_scheduler Notice simplenews_scheduler.module line 534

Thanks for starting with this.

The coding style will need some work, there are lots of trailing spaces and comments that need to follow the standards.

+++ b/tests/simplenews_scheduler.test
@@ -0,0 +1,90 @@
+      'description' => 'Testing generation of newsletters',
+      'group' => 'Simplenews Scheduler',
+    );

I think it makes sense to put this into the existing Simplenews group... there are already way too many groups on that page.

+++ b/tests/simplenews_scheduler.test
@@ -0,0 +1,90 @@
+    $edit["simplenews[send]"] = '4';

You might want to use the defined constant here (should be in the .module file), that makes the code easier to understand.

Will also use a few more test assertions and we'll need to make a few combinations of different times to check that it's generating only when it's supposed to. For example, you might want to start with a time that shouldn't actually generate anything and only then have it generate one. Because right now, the tests would pass just fine if the module would generate a newsletter every time cron is called :)

Not sure about the notice, maybe that just needs a check if the property exists.

My main concern is why i'm getting that notice, everything else I can manage, but i'm not so motivated to continue until i get that notice figured out, ideas?

Looks like this line is wrong, maybe a left-over:

      'pid' => $edition_node->simplenews_scheduler->nid,

My guess is that needs to look like this instead:

      'pid' => $template_node->nid,

If you want to discuss anything, please ping me in #drupal-contribute

There's a couple of patches I was holding off on committing to not break the D7 patch, which refactor the code with a view to making it a lot easier to write tests for:

- #1403830: refactor hook_cron to aid testing
- #1399626: further refactoring in and around _simplenews_scheduler_new_edition()

Now that the D7 port patch is in, I can commit these.

OK cool, i have committed this one, thanks Berdir!

shall we backport to D6?