On 2019-10-12 07:47, Daniel Shahaf wrote:
> Yasuhito FUTATSUKI wrote on Sat, Oct 12, 2019 at 05:31:53 +0900:
>> Yes, it will fix local_missing_dir_endless_loop() itself correctly.
>> But the stack trace before fix indicate there is at least one problem
>> in svntest.verify.compare_and_display_lines().
>>
>> Assume the file contents is broken here. This situation can be simulate
>> by patch like:
>>
>> Index: subversion/tests/cmdline/tree_conflict_tests.py
>> ===================================================================
>> --- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
>> +++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
>> @@ -1544,7 +1544,7 @@
>> contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
>> svntest.verify.compare_and_display_lines(
>> "A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
>> - [ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
>> + [ b"This is the file 'lambda'.\n", b"This is not more content.\n"], contents)
>> #######################################################################
>>
>>
>> then we will got fails.log, contains stack trace for unexpected exception
>> within the code to construct log message.
>>
>> [[[
>> W: A1/B/lambda has unexpectected contents
>> W: EXPECTED svn-test-work/working_copies/tree_conflict_tests-26/A1/B/lambda (match_all=True):
>> W: CWD: /home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline
>> Traceback (most recent call last):
>> File "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/main.py", line 1931, in run
>> rc = self.pred.run(sandbox)
>> File "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/testcase.py", line 178, in run
>> result = self.func(sandbox)
>> File "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/tree_conflict_tests.py", line 1547, in local_missing_dir_endless_loop
>> [ b"This is the file 'lambda'.\n", b"This is not more content.\n"], contents)
>> File "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py", line 503, in compare_and_display_lines
>> expected.display_differences(message, label, actual)
>> File "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py", line 154, in display_differences
>> display_lines(message, self.expected, actual, e_label, label)
>> File "/home/futatuki/work/subversion/vwc/trunk/subversion/tests/cmdline/svntest/verify.py", line 474, in display_lines
>> logger.warn('| ' + x.rstrip())
>> TypeError: can only concatenate str (not "bytes") to str
>> FAIL: tree_conflict_tests.py 26: endless loop when resolving local-missing dir
>> ]]]
>>
>> This is caused by mixing bytes object drived from file contents and str
>> object to construct log message.
>
> I agree: this FAIL indicates str and bytes are mixed. My question is:
> Why do you think svntest.verify.compare_and_display_lines() needs to be
> changed? That function's names implies it deals with text files, so,
> why should compare_and_display_lines() support the case that its third
> and fourth parameters (EXPECTED and ACTUAL) are both lists of bytes objects?
>
> In other words, why would changing «'rb'» to «'r'» on line 1544 —
> without changing the str literals to bytes literals on line 1547 — not
> be a correct solution?
Ah, I thought strict comparison is needed here because of raw mode file I/O.
Index: subversion/tests/cmdline/tree_conflict_tests.py
===================================================================
--- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
+++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
@@ -1518,7 +1518,7 @@
sbox.simple_move('A/B', 'A/B2')
sbox.simple_commit()
sbox.simple_update()
- main.file_append_binary(sbox.ospath("A/B2/lambda"), "This is more content.\n")
+ main.file_append_binary(sbox.ospath("A/B2/lambda"), "This is more content.\r\n")
sbox.simple_commit()
sbox.simple_update()
@@ -1544,7 +1544,7 @@
contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
svntest.verify.compare_and_display_lines(
"A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
- [ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
+ [ b"This is the file 'lambda'.\n", b"This is more content.\n"], contents)
#######################################################################
Above patch will make local_missing_dir_endless_loop test fail because of
strictness of comparison. However,
Index: subversion/tests/cmdline/tree_conflict_tests.py
===================================================================
--- subversion/tests/cmdline/tree_conflict_tests.py (revision 1868264)
+++ subversion/tests/cmdline/tree_conflict_tests.py (working copy)
@@ -1518,7 +1518,7 @@
sbox.simple_move('A/B', 'A/B2')
sbox.simple_commit()
sbox.simple_update()
- main.file_append_binary(sbox.ospath("A/B2/lambda"), "This is more content.\n")
+ main.file_append_binary(sbox.ospath("A/B2/lambda"), "This is more content.\r\n")
sbox.simple_commit()
sbox.simple_update()
@@ -1541,7 +1541,7 @@
# If everything works as expected the resolver will recommended a
# resolution option and 'svn' will resolve the conflict automatically.
# Verify that 'A1/B/lambda' contains the merged content:
- contents = open(sbox.ospath('A1/B/lambda'), 'rb').readlines()
+ contents = open(sbox.ospath('A1/B/lambda'), 'r').readlines()
svntest.verify.compare_and_display_lines(
"A1/B/lambda has unexpectected contents", sbox.ospath("A1/B/lambda"),
[ "This is the file 'lambda'.\n", "This is more content.\n"], contents)
this will make it succcess because of "universal newline" feature on Python.
If textual comparison is sufficient here, it is right to open file
text mode (with suitable, unified set of `encoding', `errors', and `newline'
parameter). Otherwise, if strict comparison is needed, we must avoid unwanted,
not one-on-one corresponding conversion from bytes to str applied by Python.
In the latter case, it may be rather incorrect to use
compare_and_display_lines().
> Hope I'm clearer this time. If not, I'd be happy to clarify.
>
> Cheers,
>
> Daniel
>
Cheers,
--
Yasuhito FUTATSUKI <futatuki_at_poem.co.jp>
Received on 2019-10-12 05:02:24 CEST