According to http://drupal.org/node/383600#comment-1392166 we should move all test files accept aggregator.test into a subdirectory tests/

For aggregator, these are:

aggregator_test.module
aggregator_test.info
aggregator_test_rss091.xml

This is a novice task.

Comments

JamesAn’s picture

Status: Active » Needs review
StatusFileSize
new8.97 KB

This patch moves the 3 files into a test folder.

Also, line 53 of aggregator_test.module refers to the path of aggregator_test_rss091.xml, which is changed (note the addition of '/test'):
$file_name = DRUPAL_ROOT . '/' . drupal_get_path('module', 'aggregator') . '/test/aggregator_test_rss091.xml';

JamesAn’s picture

StatusFileSize
new8.98 KB

Oy vey. I got to folder name wrong. tests, not test.

Here's the fixed version.

Status: Needs review » Needs work

The last submitted patch failed testing.

JamesAn’s picture

StatusFileSize
new9.65 KB

Ok, ok.. still relatively new to this patching thing. I figured out how to run the tests so I can confirm that they work. Here's the correct patch.

#408982: Skip syntax check if file doesn't exist points out the bug in PIFR, where patches that delete files throw a "Failed: Invalid PHP syntax" error as PIFR attempts to execute the deleted file anyway.

So this patch will still "fail" as it deletes the existing test files and creates new ones in the tests folder, but it does work. I think. ^_^"

aron novak’s picture

I can confirm that the tests work.

JamesAn: did you follow "If you are using -N to account for a deleted file, you will also need to edit the CVS/Entries file for that directory" section at http://drupal.org/patch/create ?
If yes, it seems that it's out of our scope.
(edit: i saw that the deleted files are fine in the pach)

JamesAn’s picture

Yes, I edited the CVS/Entries file as per http://drupal.org/patch/create instructions for creating and deleting the files. I was wondering how those edits would be reflected since they're not wrapped in the patch.

I created a similar issue for the test files in the node module: #418760: Move test files into tests/. The patch was created in the same way. Can you confirm that one too? Thanks!

aron novak’s picture

Status: Needs work » Reviewed & tested by the community

The patch works nice, so it's RTBC now.

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

JamesAn’s picture

Status: Needs work » Needs review

Hmm.. does setting the status back to needs review schedule a retest? Only one way to find out!

dave reid’s picture

Status: Needs review » Reviewed & tested by the community

Looks good and now we can add/remove files and the test bot won't complain! Yay! 10687 passes is the current magic number so all the tests ran.

dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Novice

Automatically closed -- issue fixed for 2 weeks with no activity.