RapidPro: Issue error if router comparison arguments are empty strings #164
Conversation
|
We should always be creating valid output, but if we absolutely cannot we should fail. If the output is not useful (even if it is valid), we should also fail. |
src/rpft/rapidpro/models/routers.py
Outdated
| @@ -202,6 +202,7 @@ def generate_category_name(self, comparison_arguments): | |||
| # Auto-generate a category name that is guaranteed to be unique | |||
| # TODO: Write tests for this | |||
There was a problem hiding this comment.
This todo comment was written over 4 years ago - I think it's time it was removed. Personally, I would only use todos when work is in progress and remove them before merging.
There was a problem hiding this comment.
Fair enough, I guess it's not going to happen (and not important enough to capture in an issue?)
src/rpft/rapidpro/models/routers.py
Outdated
| @@ -537,8 +542,8 @@ def validate(self): | |||
| raise ValueError(f'Invalid router test type: "{self.type}"') | |||
| if not RouterCase.TEST_VALIDATIONS[self.type](self.arguments): | |||
| print( | |||
There was a problem hiding this comment.
We should use the logger, or raise an exception, rather than print.
There was a problem hiding this comment.
Agreed. Streamlined the use of exceptions here. (I'm not using the logger in the rapidpro sublibrary.)
Errors like this are not covered by the test suite, so please test manually that the error messages are as desired.
fixes #160