test: Add tests for case sensitivity #6194
Conversation
This also includes changes to existing tests to deal with case-sensitivity issues that were revealed by temporarily hard-wiring PersonFactory to use an upper-case or mixed-case email address.
Codecov Report
@@ Coverage Diff @@
## main #6194 +/- ##
=======================================
Coverage 88.68% 88.69%
=======================================
Files 290 290
Lines 40300 40390 +90
=======================================
+ Hits 35742 35824 +82
- Misses 4558 4566 +8 |
There was a problem hiding this comment.
It looks to me like something is missing - see the comment about do_submission().
There's an inline suggestion about keyword-only arguments. That's more a question about whether you think it'd be appropriate than a demand - we haven't used keyword-only args in the code so far, but I think it's something we should consider.
| @@ -237,7 +237,7 @@ def create_and_post_submission(self, name, rev, author, group=None, formats=("tx | |||
| self.assertTrue(os.path.exists(os.path.join(self.staging_dir, "%s-%s.%s" % (name, rev, 'html')))) | |||
| return r | |||
|
|
|||
| def do_submission(self, name, rev, group=None, formats: Tuple[str, ...]=("txt",), author=None, base_filename=None, ascii=True): | |||
| def do_submission(self, name, rev, group=None, formats: Tuple[str, ...]=("txt",), author=None, email=None, base_filename=None, ascii=True): | |||
There was a problem hiding this comment.
I should probably have caught this the first time around, but I don't think this email parameter is used either. Worries me that the test is not testing what it means to test.
|
Closing this as OBE. We can come back to it as part of a future cleanup effort. |
This also includes changes to existing tests to deal with case-sensitivity issues that were revealed by temporarily hard-wiring PersonFactory to use an upper-case or mixed-case email address.
This testing was done while researching #5929 and #6057. This testing didn't result in code changes outside of the test code, so it's not imperative that it be merged.