From dev-return-39555-archive-asf-public=cust-asf.ponee.io@subversion.apache.org Sat Oct 12 03:02:24 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id BC33C180656 for ; Sat, 12 Oct 2019 05:02:23 +0200 (CEST) Received: (qmail 65472 invoked by uid 500); 12 Oct 2019 03:02:22 -0000 Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@subversion.apache.org Received: (qmail 65437 invoked by uid 99); 12 Oct 2019 03:02:20 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 12 Oct 2019 03:02:20 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id E5E1AC117E for ; Sat, 12 Oct 2019 03:02:19 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 0.799 X-Spam-Level: X-Spam-Status: No, score=0.799 tagged_above=-999 required=6.31 tests=[KAM_ASCII_DIVIDERS=0.8, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Received: from mx1-ec2-va.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id SSXXec7D48ox for ; Sat, 12 Oct 2019 03:02:16 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=124.35.235.17; helo=poem.co.jp; envelope-from=futatuki@poem.co.jp; receiver= Received: from poem.co.jp (sv.poem.co.jp [124.35.235.17]) by mx1-ec2-va.apache.org (ASF Mail Server at mx1-ec2-va.apache.org) with ESMTPS id AA4D0BC67C for ; Sat, 12 Oct 2019 03:02:15 +0000 (UTC) Received: from retina-alpha.yf.bsdclub.org (FL1-220-144-210-154.iba.mesh.ad.jp [220.144.210.154]) (authenticated bits=0) by poem.co.jp (8.13.8/8.13.8) with ESMTP id x9C31nuX006337 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Sat, 12 Oct 2019 12:01:55 +0900 Subject: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)) To: Daniel Shahaf Cc: Nathan Hartman , Subversion Developers , Julian Foad References: <20191010205444.srzggabby2en27xg@tarpaulin.shahaf.local2> <9e97a2ad-854b-4cb3-af3a-b80c6a14fa0b@www.fastmail.com> <20191011175601.y4rlv62tvmbcxkjn@tarpaulin.shahaf.local2> <631611e4-0eb3-c3cc-b087-cc33dacc20bb@poem.co.jp> <20191011224709.reoh6tycyk3nyhsb@tarpaulin.shahaf.local2> From: Yasuhito FUTATSUKI Message-ID: Date: Sat, 12 Oct 2019 12:01:34 +0900 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20191011224709.reoh6tycyk3nyhsb@tarpaulin.shahaf.local2> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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