Fix error when removing uploaded post breach file on config reset#1054
Conversation
Codecov Report
@@ Coverage Diff @@ ## develop #1054 +/- ## =========================================== - Coverage 29.13% 29.01% -0.13% =========================================== Files 410 409 -1 Lines 12934 12912 -22 =========================================== - Hits 3768 3746 -22 Misses 9166 9166
Continue to review full report at Codecov.
|
shreyamalviya force-pushed the bugfix_pba_file_delete branch from 8ef0986 to f61f9ed Compare shreyamalviya force-pushed the bugfix_pba_file_delete branch from f61f9ed to 1ad8249 Compare 
… another function
shreyamalviya force-pushed the bugfix_pba_file_delete branch from f84cdac to 4f94e9d Compare 
| @mssalvatore I'm not sure how much value we'll get out of adding tests for this. |
Had we had the tests in the first place, this bug might have been prevented. Where possible, we should try to first write a test that reproduces the issue, then fix it and see that the test passes. A set of tests for this function seems feasible. It's also a good opportunity to investigate the best way to test our API endpoints. |
| (PBA_LINUX_FILENAME_PATH if is_linux else PBA_WINDOWS_FILENAME_PATH), filename | ||
| ) | ||
| return filename | ||
| |
There was a problem hiding this comment.
I would recommend putting this method immediately after the one that calls it so as to follow the Stepdown rule
| @@ -69,13 +69,10 @@ def delete(self, file_type): | |||
| PBA_LINUX_FILENAME_PATH if file_type == "PBAlinux" else PBA_WINDOWS_FILENAME_PATH | |||
There was a problem hiding this comment.
Regarding the value of adding tests, does this function behave in a sane way if I pass an invalid value for file_type?
…load.py` to follow stepdown rule
2 participants
Fixes:
os.remove(file_path)inmonkey_island/cc/resources/pba_file_upload.pyattempts to delete a directory which throws an error when the PBA_linux_filename/PBA_windows_filename field in the database is empty.To reproduce: