Ransomware quickstart - Add get method for island mode #1320
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1320 +/- ##
============================================
- Coverage 59.89% 40.30% -19.59%
============================================
Files 147 471 +324
Lines 4787 13823 +9036
============================================
+ Hits 2867 5571 +2704
- Misses 1920 8252 +6332
Continue to review full report at Codecov.
|
|
|
||
|
|
||
| @pytest.mark.parametrize("mode", ["ransomware", "advanced"]) | ||
| def test_island_mode_get(flask_client, uses_database, mode): |
There was a problem hiding this comment.
this also tests the post. Maybe we don't need the test_island_mode_post__set_model and test_island_mode_post__set_invalid_model tests anymore?
There was a problem hiding this comment.
Tests should be individual i.e. testing one kind of behaviour. You should set IslandMode.objects and then test just the post request.
There was a problem hiding this comment.
There's no need to make unit tests too granular. The result of the post is that get returns the mode posted. There's no need to tie the unit test to the mechanism of how the mode is saved. If we decide to switch the database we won't need to re-do unit tests, for example.
There was a problem hiding this comment.
I don't agree. If the test fails, you're now left debugging more than if the tests were granular.
I don't think our motive behind writing unit tests should be to not have to redo them ever again. It should be to make debugging and catching issues easier.
|
|
||
|
|
||
| @pytest.mark.parametrize("mode", ["ransomware", "advanced"]) | ||
| def test_island_mode_get(flask_client, uses_database, mode): |
There was a problem hiding this comment.
Tests should be individual i.e. testing one kind of behaviour. You should set IslandMode.objects and then test just the post request.
2d1a3ef to
56b5e8b
Compare
What does this PR do?
Fixes #1241 get mode endpoint.
Add any further explanations here.
PR Checklist
Was the CHANGELOG.md updated to reflect the changes?Was the documentation framework updated to reflect the changes?Testing Checklist
If applicable, add screenshots or log transcripts of the feature workingExplain Changes
Are the commit messages enough? If not, elaborate.