Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I am not sure this is an issue, but i have been designing a custom module for a client that creates a directory inside the hook_install function. When I run my test it returns that the folder is there, but not writeable. Which in turn makes the file upload return the error message "The selected file %file could not be uploaded, because the destination %directory is not properly configured.".
Putting :
$path = file_directory_path().'/pdf_templates';
file_check_directory($path, FILE_CREATE_DIRECTORY);
Directly into the test function or the setUp function solves the problem.
Again, I am not sure if this is a problem, but I thought I'd post it.
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | simpletest_folder_test_with_move_2.patch | 2.38 KB | jrbeeman |
| #19 | simpletest_folder_test_with_move_1.patch | 2.38 KB | Berdir |
| #15 | simpletest_folder_test.patch | 1.09 KB | sndev |
| #15 | simpletest_folder_test_with_move.patch | 2.4 KB | sndev |
| #12 | simpletest_folder_test.patch | 1.34 KB | sndev |
Comments
Comment #1
Dave ReidYeah this is a problem. SimpleTests sets the new file directory variables after it's run all the module installs. We should probably fix this in D7 core first then backport.
Comment #2
sndev CreditAttribution: sndev commentedComment #3
boombatower CreditAttribution: boombatower commentedI am assuming that is related to #577398: Ensure test coverage of simpletest temporary file handling or should be included if not.
Comment #4
Dave ReidThis isn't actually a duplicate of it. SimpleTest module has always set the files directory variable after modules have been installed, even before the File API patch landed. I think we need a new issue for core to fix this and backport it.
Comment #5
Dave ReidTagging since XML sitemap module maintains a workaround in its test helper's setUp functions until this is fixed.
Comment #6
Dave ReidForgot this tag as well.
Comment #7
Dave ReidComment #8
sndev CreditAttribution: sndev commentedI moved the variable_set calls for the file directories before the initial profile module install. I have tested it somewhat in D6, but don't really have a use case in D7.
Comment #9
c960657 CreditAttribution: c960657 commentedI think this looks fine. Did some rudimentary testing with D7.
Comment #10
Anonymous (not verified) CreditAttribution: Anonymous commented+1 from me too, don't know why we didn't see this earlier.
perhaps add a comment in there stating why we need to do this early?
Comment #11
webchickI think a comment isn't really necessary here (never thought you'd hear me say THAT huh?!). I can't imagine people moving this block of code around willy-nilly.
What would be nice is a test for this use case, though. Then they *for sure* won't move the code around willy-nilly. ;)
Also, this smells a bit edge-casey to me to be critical.
Comment #12
sndev CreditAttribution: sndev commentedI wrote a test that uses the image module as a guinea pig. The problem only occurs if the folders/files are installed during setUp. This doesn't occur in most core modules, but image, simpletest and a bunch of contribs do utilize the functionality. I don't think the test is in the right place, but going through the simpletest/modules folder is a bit beyond me.
Comment #13
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #15
sndev CreditAttribution: sndev commentedMy last test was a little over zelous. running the uninstall modules function brought up a db error regarding uncreated tables. Lets try something a bit simpler.
Comment #16
sndev CreditAttribution: sndev commentedComment #18
grendzy CreditAttribution: grendzy commentedThis affects imagecache too.
Comment #19
BerdirThis just failed because there was a "." added to the comment below, This is just a re-roll.
Comment #20
gowriabhaya CreditAttribution: gowriabhaya commentedCode sprint tag
Comment #21
jrbeemanPatch successfully applied to a fresh CVS checkout and test passes. Minor cosmetic change made to patch ("Directory Created" to "Directory created").
Comment #22
Dave ReidLooks great and I've been using this patch for a while now on core with XML sitemap tests, which create a folder on install.
Comment #23
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.