Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 0.3.1
Bug #19030 running test suite modifies data/IS_postcodes.txt (for PEAR QA team)
Submitted: 2011-11-15 01:59 UTC
From: danielc Assigned: doconnor
Status: Closed Package: Validate_IS
PHP Version: 5.4.0alpha1 OS:
Roadmaps: (Not assigned)    
Subscription  


 [2011-11-15 01:59 UTC] danielc (Daniel Convissor)
Description: ------------ Hi: Executing the test suite winds up modifying the data/IS_postcodes.txt file. Thanks, --Dan

Comments

 [2011-11-15 02:02 UTC] danielc (Daniel Convissor)
Added patch the-data-that-gets-modified.diff.
 [2011-12-07 09:34 UTC] danielc (Daniel Convissor)
Can you please take a look at this so we can improve the continuous integration systems for PEAR and PHP?
 [2011-12-10 08:15 UTC] doconnor (Daniel O'Connor)
-Package: Validate +Package: Validate_IS
 [2011-12-10 18:32 UTC] doconnor (Daniel O'Connor)
-Status: Open +Status: Feedback
Thank you for taking the time to report a problem with the package. This problem may have been already fixed by a previous change that is in the SVN of the package. Please checking out the SVN repository of this package and upgrade svn checkout svn.php.net/repository/pear/packages/Validate_IS/trunk pear upgrade package2.xml or pear upgrade package.xml If you are able to reproduce the bug with the latest SVN, please change the status back to "Open". Again, thank you for your continued support of PEAR. Uhm... https://github.com/pear/Validate_IS/blob/master/data/IS_postcodes.txt#L136 looks like it's already here?
 [2011-12-10 20:08 UTC] danielc (Daniel Convissor)
The data is in there from the move to GitHub. But that's not the problem. Tthe postalCode() method has the capability to retrieve live data from the Icelandic postal service's website. The "Test postalCode strong" section of validate_IS.phpt uses that functionality and writing the output to data/IS_postcodes.txt. Having the test suite modifying the data feels very wrong to me. First, it prevents non-root users from running the test suite on the installed package because they won't have write privileges on the data directory. Second, changing the file can cause the test suite to think everything is okay because it's giving the data it wants to find. And since the test passes, there is no indication that the original data file needs to be committed and put into a release for public consumption.
 [2011-12-10 20:09 UTC] danielc (Daniel Convissor)
-Status: Feedback +Status: Open
 [2011-12-10 21:28 UTC] bjori (Hannes Magnusson)
Well.. how would you update the test to test the strong method without doing the on- the-fly update? I do agree that a unittest shouldn't modify data files, but the whole point with that argument is to do live check and update the dataset.
 [2011-12-10 21:32 UTC] danielc (Daniel Convissor)
How about having the "strong" test write to a temporary file?
 [2012-03-25 19:17 UTC] doconnor (Daniel O'Connor)
-Status: Open +Status: Closed -Assigned To: +Assigned To: doconnor
This bug has been fixed in SVN. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better.