-
Notifications
You must be signed in to change notification settings - Fork 80
delete sample template #1059
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
delete sample template #1059
Changes from all commits
1e3a802
e9d3345
4e6099a
e2a5558
62e1aff
b47cbd5
1a6762d
a87fc2f
3a76380
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -382,5 +382,59 @@ class TestEBISubmitHandler(TestHandlerBase): | |
pass | ||
|
||
|
||
class TestDelete(TestHandlerBase): | ||
database = True | ||
|
||
def test_delete_sample_template(self): | ||
response = self.post('/study/description/1', | ||
{'sample_template_id': 1, | ||
'action': 'delete_sample_template'}) | ||
self.assertEqual(response.code, 200) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you also make sure that the sample template is actually deleted here? Same goes for all other delete tests. Just want to make sure the functionality is actually happening along with the page loading fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure this is the right place for that as it already in qiita_db but I
can add if you feel really strong about it. Note that this will change the
current code and add some complexity, for example: you can not erase a
study template that has prep template so you need to cascade the deletion
...
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh no, this isn't to make sure the delete function works, just that the delete page actually calls delete and the items are actually deleted when you hit the buttons on the page. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps I'm missing something, what's the difference (an example will be great)?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just because the delete function works in qiita_db, doesn't mean the call works in qiita_pet. The delete function may change, breaking qiita_pet, or some other thing may change. If you test in qiita pet that the deletion actually happens, you know the functionality always works. As an example, in test_delete_raw_data, you can make sure that the QiitaDBIDError is raised when instantiating RawData(1) to make sure it was deleted by the page call. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Concrete example here Where the database change called for in the page is explicitly tested that it happens. |
||
|
||
# checking that the action was sent | ||
self.assertIn("Sample template can not be erased because there are " | ||
"raw datas", response.body) | ||
|
||
def test_delete_raw_data(self): | ||
response = self.post('/study/description/1', | ||
{'raw_data_id': 1, | ||
'action': 'delete_raw_data'}) | ||
self.assertEqual(response.code, 200) | ||
|
||
# checking that the action was sent | ||
self.assertIn("Raw data 1 has prep template(s) associated so it can't " | ||
"be erased", response.body) | ||
|
||
def test_delete_prep_template(self): | ||
response = self.post('/study/description/1', | ||
{'prep_template_id': 1, | ||
'action': 'delete_prep_template'}) | ||
self.assertEqual(response.code, 200) | ||
|
||
# checking that the action was sent | ||
self.assertIn('Cannot remove prep template 1 because a preprocessed ' | ||
'data has been already generated using it.', | ||
response.body) | ||
|
||
def test_delete_preprocessed_data(self): | ||
response = self.post('/study/description/1', | ||
{'preprocessed_data_id': 1, | ||
'action': 'delete_preprocessed_data'}) | ||
self.assertEqual(response.code, 200) | ||
|
||
# checking that the action was sent | ||
self.assertIn('Illegal operation on non sandboxed preprocessed data', | ||
response.body) | ||
|
||
def test_delete_processed_data(self): | ||
response = self.post('/study/description/1', | ||
{'processed_data_id': 1, | ||
'action': 'delete_processed_data'}) | ||
self.assertEqual(response.code, 200) | ||
|
||
# checking that the action was sent | ||
self.assertIn('Illegal operation on non sandboxed processed data', | ||
response.body) | ||
|
||
if __name__ == "__main__": | ||
main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this tests should include
database = True
; since they can potentially modify the database, in case of malfunction; making a lot of tests to fail and harder to find the original problem.