Some questions inline
Diff comments:
> diff --git a/lib/lp/bugs/utilities/tests/test_filebugdataparser.py
> b/lib/lp/bugs/utilities/tests/test_filebugdataparser.py
> index 28d4be9..84b8e0b 100644
> --- a/lib/lp/bugs/utilities/tests/test_filebugdataparser.py
> +++ b/lib/lp/bugs/utilities/tests/test_filebugdataparser.py
> @@ -44,9 +67,65 @@ class TestFileBugDataParser(TestCase):
> self.assertEqual(b"", parser._consumeBytes(b"0"))
> self.assertEqual(b"", parser._consumeBytes(b"0"))
>
> + def test__findLineBreakType(self):
> + # _findLineBreakType reads the whole message until either an LF or
> + # a CRLF is found, returns that type in bytes string format and
> exits.
> +
> + msg = dedent(
> + """\
> + Header: value
> + Space-Folded-Header: this header
> + is folded with a space.
> + Tab-Folded-Header: this header
> + \tis folded with a tab.
> + Another-header: another-value
> +
> + Not-A-Header: not-a-value
> + """
> + )
> +
> + # Test the above message with LF endings
> + msg_with_lf = msg.encode("ASCII")
> + lf_parser = FileBugDataParser(MockBytesIO(msg_with_lf))
> + lf_linebreak = lf_parser._findLineBreakType()
> +
> + self.assertEqual(lf_linebreak, b"\n")
Can we add a warning if you do use \n and test that the warning is sent? Or
will that be part of another MP?
> +
> + # Test the above message after replacing LF with CRLF
> + msg_with_crlf = msg.replace("\n", "\r\n").encode("ASCII")
> + crlf_parser = FileBugDataParser(MockBytesIO(msg_with_crlf))
> + crlf_linebreak = crlf_parser._findLineBreakType()
> +
> + self.assertEqual(crlf_linebreak, b"\r\n")
> +
> + # Test the above message after deleting the line-breaks
> + # Which should break as there must be at least some line-breaks
> + # in the message
Are we sure there are always line breaks in the message? I guess yes because of
headers right?
> + msg_without_linebreak = msg.replace("\n", "").encode("ASCII")
> + without_linebreak_parser = FileBugDataParser(
> + MockBytesIO(msg_without_linebreak)
> + )
> +
> + self.assertRaisesWithContent(
> + AssertionError,
> + "There are no linebreaks in the blob.",
> + without_linebreak_parser._findLineBreakType,
> + )
> +
> + # Test the above message after replacing LF with CR
> + # Which should break as we do not accept CR line-breaks
> + msg_with_cr = msg.replace("\n", "\r").encode("ASCII")
> + cr_parser = FileBugDataParser(MockBytesIO(msg_with_cr))
> +
> + self.assertRaisesWithContent(
> + AssertionError,
> + "The wrong linebreak is used. CR isn't accepted.",
> + cr_parser._findLineBreakType,
> + )
> +
> def test_readLine(self):
> # readLine reads a single line of the file.
> - parser = FileBugDataParser(io.BytesIO(b"123\n456\n789"))
> + parser = FileBugDataParser(MockBytesIO(b"123\n456\n789"))
> self.assertEqual(b"123\n", parser.readLine())
> self.assertEqual(b"456\n", parser.readLine())
> self.assertEqual(b"789", parser.readLine())
--
https://code.launchpad.net/~ilkeremrekoc/launchpad/+git/launchpad/+merge/481785
Your team Launchpad code reviewers is requested to review the proposed merge of
~ilkeremrekoc/launchpad:accept-crlf-bugreports into launchpad:master.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
