Closed (fixed)
Project:
Drupal core
Version:
7.x-dev
Component:
aggregator.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
29 Mar 2009 at 19:05 UTC
Updated:
3 Jan 2014 at 00:07 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
JamesAn commentedThis 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';Comment #2
JamesAn commentedOy vey. I got to folder name wrong. tests, not test.
Here's the fixed version.
Comment #4
JamesAn commentedOk, 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. ^_^"
Comment #5
aron novakI 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)
Comment #6
JamesAn commentedYes, 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!
Comment #7
aron novakThe patch works nice, so it's RTBC now.
Comment #9
JamesAn commentedHmm.. does setting the status back to needs review schedule a retest? Only one way to find out!
Comment #10
dave reidLooks 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.
Comment #11
dries commentedCommitted to CVS HEAD. Thanks!