SimpleTest proper installation of test files and removal
drewish - September 6, 2008 - 23:17
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | simpletest.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | boombatower |
| Status: | closed |
Description
I was running into problems getting the DrupalWebTestCase::drupalGetTestFiles() to return images. Turns out that the installer wasn't copying all the files.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| simpletest_install.patch | 1.19 KB | Idle | Failed: Failed to apply patch. | View details |
#1
I should add, because one of my tests had deleted the existing files.
#2
Never noticed I am assuming because I had files install correctly once. I also added deletion of files and folder during un-install along with comments.
Upload test passes and after fresh install of SimpleTest (deleted files) it seems to work.
#3
Sorry, could I please have some test instructions with this patch?
#4
Should uninstall SimpleTest and make sure that the files/simpletest directory doesn't exists. (it won't remove them without patch, so you will have to do manually)
Install SimpleTest and make sure that it puts all the files there and upon uninstall it removes them.
#5
Guess I should change back to RTBC?
#6
As for testing the current broken behavior delete half of the simpletest image files. That will cause it to avoid installing the rest of them.
#7
I confirmed that the uninstall stuff does what's intended. I still don't quite grok the copy stuff. That probably means it needs to be better documented than "// Copy other test files for consistency." Is that overwriting the files in the files directory with whatever's in simpletest's files directory, or..?
#8
webchick, the problem is that some of the files are generated and some are copied. the generated ones work fine but the current test for the copied ones is faulty. if there is a single html, image, javascript, php, or sql file then none will be copied. I changed the test to copy them if there are fewer. Perhaps it not even worth testing, maybe we should always just overwrite them.
#9
Added extra comment.
#10
Re-rolled.
#11
Shoot. :( No longer applies. Sorry. :(
#12
Re-rolled.
#13
Attached patch correct typo :
+ // Unistall schema.#14
+ // If there are more files in the original directory then the files directory, copy files."then" is also a typo. It should be "than".
#15
Forgot to change status.
#16
Re-rolled...
#17
at first i was going to call it a bug that this changed file_unmanaged_copy() to file_copy() but the more i think about it, i think it's a good idea. it'll make it easier to fully test the file system since the files will already exists in the db so if i want to search for something by mime type it's easy to do so.
#18
ping...ping
#19
still applies...
#20
Oops, I thought I committed this already!
Committed to HEAD. Thanks. :)
#21
Bzzzt. Spoke too soon.
hook_uninstall() appears to be b0rked.
I get a bazillion of these:
# Warning: preg_match(): No ending delimiter '.' found in file_scan_directory() (line 1294 of /Applications/MAMP/htdocs/core/includes/file.inc).And then:
# Warning: rmdir(sites/default/files/simpletest): Directory not empty in simpletest_uninstall() (line 112 of /Applications/MAMP/htdocs/core/modules/simpletest/simpletest.install).And so does hook_install() for that matter:
Warning: preg_match(): Unknown modifier '-' in file_scan_directory() (line 1294 of /Applications/MAMP/htdocs/core/includes/file.inc).x40 or so.
Not sure if this patch needs work or if it's just waiting on #74645: modify file_scan_directory to include a regex for the nomask. to get re-committed, but it's definitely NOT working, so rolled it back. Sorry.
#22
ah, actually the patch hadn't been updated for the file_scan_directory() preg regex change. it also was also calling the old file_copy() and file_delete() rather than the unmanged versions.
to test this, disable simple test and then when uninstalling make sure it removes the files/simpletest directory, re-install the module and make sure all the files are copied/generated. then repeat the uninstall process.
#23
The last submitted patch failed testing.
#24
issue with core.
#25
The last submitted patch failed testing.
#26
#27
Good comments; even I can understand the patch now. ;)
I assume there is no way for the destination to contain files /not/ in the original path, which would make the file-count comparison unreliable. Common sense says we can always assume that, but I'm mentioning it just in case.
#28
We should be able to assume that, but otherwise it could do a comparison of the files themselves.
#29
This looks good. However, I'd add one sentence to
+ // If there are more files in the original directory than the files directory, copy files.explain _when_ or _why_ this can happen.#30
This behaves like the "Clean environment" button. More for automatically fixing any issues with the test environment.
The uninstall behavior is technically a bug since it should not leave all the test files behind.
#31
+ // If there are more files in the original directory than the files+ // directory, copy files. This can occur when test files are deleted from
+ // the SimpleTest directory. It servers a convenience to developers so that
+ // they can get the files back easily.
"servers" sounds like a typo.
So, right now, according to the documentation, there are 3 directories: the original directory, the files directory and the SimpleTest directory. I initially thought that the original directory was the SimpleTest directory but that doesn't make sense when I read the code comments. It is still not clear how files can disappear from the original directory?
#32
"servers" => "serves"
The SimpleTest directory stuff was a messup. Not sure how I managed that. :)
#33
This looks good to me -- it is reasonably clear now. Let's see what others have to say.
#34
I'm not going to change the status on this since I think this patch is totally RTBC but I was going to offer up an alternate comment:
If there are more files in SimpleTest's files directory than the site's filesdirectory, restore all the files. This situation might occur when an errant test
deletes one or more files from the site's files directory. It serves a
convenience to developers so that they can get the test files back easily.
#35
I like the comment.
#36
Great. Thanks. Committed! Next patch? :)
#37
Automatically closed -- issue fixed for 2 weeks with no activity.