On 2020/05/07 16:17, Johan Corveleyn wrote:
> On Wed, May 6, 2020 at 10:36 AM Yasuhito FUTATSUKI <futatuki_at_poem.co.jp> wrote:
>>
>> On 2020/05/04 4:49, Johan Corveleyn wrote:
>>> On Sun, May 3, 2020 at 6:23 PM Yasuhito FUTATSUKI <futatuki_at_poem.co.jp> wrote:
>>>>
>>>> On 2020/05/03 10:02, Johan Corveleyn wrote:
>>>>> On Fri, May 1, 2020 at 7:46 PM Yasuhito FUTATSUKI <futatuki_at_poem.co.jp> wrote:
>>>>>>
>>>>>> On 2020/04/23 2:05, Yasuhito FUTATSUKI wrote:
>>
>>>>>> It seems following tests are EOL style sensitive, but they didn't
>>>>>> check without strict EOL style on Windows:
>>>>>>
>>>>>> merge_tests.merge_conflict_markers_matching_eol
>>>>>> merge_tests.merge_eolstyle_handling
>>>>>> patch_tests.patch_no_svn_eol_style
>>>>>> patch_tests.patch_with_svn_eol_style
>>>>>> patch_tests.patch_with_svn_eol_style_uncommitted
>>>>>> update_tests.conflict_markers_matching_eol
>>>>>> update_tests.update_eol_style_handling
>>>>>>
>>>>>> I've not check that each tests depend that native EOL is '\n' or not.
>>>>>> However, I think if some tests depend on it, other test cases for
>>>>>> those aims are needed on Windows, or at least explicitly skip the tests
>>>>>> on Windows to clarify that those tests check nothing for the aims.
>>>>>>
>>>>>> The updated patch attached only make these tests EOL style sensitive
>>>>>> (and relax check of EOL on Python 2 if keep_eol_style optional
>>>>>> parameter is not specified).
>>>>>
>>>>> Thank you for continuing to work on this, Yasuhito.
>>>>>
>>>>> This patch indeed fixes the above 7 tests when running with Python 3 on Windows.
>>>>> However, when running with Python 2 there are now 6 tests that fail
>>>>> (without this patch, all tests are successful with Py2):
>>>>
>>>> Thank you for testing.
>>>>
>>>> As far as this test result, those test cases don't depend on specific
>>>> native EOL.
>>>>
>>>>> FAIL: merge_tests.py 34: conflict markers should match the file's eol style
>>>>> FAIL: patch_tests.py 13: patch target with no svn:eol-style
>>>>> FAIL: patch_tests.py 14: patch target with svn:eol-style
>>>>> FAIL: patch_tests.py 15: patch target with uncommitted svn:eol-style
>>>>> FAIL: patch_tests.py 57: patch a binary file
>>>>> FAIL: update_tests.py 26: conflict markers should match the file's eol style
>>>>>
>>>>> See fails.log in attachment.
>>>>
>>>> I overlooked that result of io.TextIO.read() is unicode on Python 2.
>>>> I hope updated patch may resolve this issue.
>>>
>>> Okay, that latest version fixes this test for Python 2:
>>>
>>> patch_tests.py 57: patch a binary file
>>>
>>> But the five other failures still remain for Py2:
>>>
>>> FAIL: merge_tests.py 34: conflict markers should match the file's eol style
>>> FAIL: patch_tests.py 13: patch target with no svn:eol-style
>>> FAIL: patch_tests.py 14: patch target with svn:eol-style
>>> FAIL: patch_tests.py 15: patch target with uncommitted svn:eol-style
>>> FAIL: update_tests.py 26: conflict markers should match the file's eol style
>>>
>>> See fails_py2.log in attachment.
>>
>> I read those test code again, and I found that they don't distinct
>> text I/O and binary I/O when they write to files in some case, So I
>> updated the patch address them. (sain_keep_eol_style_win_patch_20200506.txt)
>
> Yes, it does :-). Perfect! With this version of the patch, all tests
> pass again with Python 2.
Thanks. I think they do what we want them to do now.
>>> For Python 3 we're getting close. After this patch and the one for
>>> svnrdump_tests, we only have 2 failures left with Python 3:
>>
>> Please don't forget our goal is not to make the tests be passed but
>> to make the tests check what we want to check, on Windows both with
>> py2 and py3.
>
> Agreed. And thank you for being so thorough. It's great that you took
> the time to analyse the tests so deeply.
>
>> I've fixed the test codes to make them what the author intended I think.
>> If the results of py2 and py3 are still different, the test code are
>> still broken. With a broken test, we can't warrant ether the test target
>> code is correct or the test case is correct, even if the tests can be
>> passed. So we should also check the test cases (scenarios and expected
>> results) themselves are correct on Windows(, and I didn't it... at
>> least, yet).
Now I've confirmed those cases themselves. It seems they don't depend on
what is EOL character(s), except value of variable "native_nl".
>>> FAIL: svnadmin_tests.py 35: detect denormalized names and name collisions
>>
>> This is caused by output message of svnadmin, containing non UTF-8
>> character. Attached patch fix_svnadmin_tests_patch.txt address it,
>> however I don't have confidence because I don't know what charset/encoding
>> svnadmin on Windows use. With Python 2, output message is treated
>> as bytes as is, this is not affected.
>
> Well, I can confirm that the test passes on my Windows 10 system with
> this patch. Both with Python 3 and Python 2. I can't comment on the
> general validity (this is a bit above my head), but it does work :-).
If svnadmin uses charset/encoding other than 'utf-8' for output messages,
this patch is incorrect, because expected strings or regular expressions
are always Uniode, which is decoded by approprite codecs for the tool.
So I still doubt this result is by chance.
>>> FAIL: svndumpfilter_tests.py 7: svndumpfilter with an empty prefix
>>
>> It seems this is just same reason as svnrdump_tests.py was broken.
>> (fix_svndumpfilter_tests_patch.txt)
>
> Yes, perfect! It works.
Thanks, I think this can safely be commited.
> Awesome work, Yasuhito! With these 3 patches applied to my trunk
> working copy, the entire testsuite now passes for me on Windows both
> with Python 2 and Python 3.
>
> So, as far as I'm concerned: please go ahead and commit those patches!
> (one tiny nit: in your log messages, please indent the texts following
> the symbols with 2 spaces, so it aligns with the bullets nicely, as in
> the examples on
> http://subversion.apache.org/docs/community-guide/conventions.html#log-messages)
>
> We should probably also propose all of your latest fixes for backport
> in /branches/1.14.x/STATUS, so they can be included in 1.14.1 (and
> interested people can already see that the "known testsuite issues on
> Windows" are being addressed).
Thank you for your advice. However I worry about Python 2 regression
mentioned in another thread. If it is true, those patch can be also
affected. So I'll wait for a detailed report.
> There is still the issue of PYTHONLEGACYWINDOWSSTDIO that is very much
> blocking for anyone running the testsuite on Windows with Python 3.
> Worst case, we might have to simply document it, and perhaps emit a
> warning if it's not set? Or can we simply set that envvar ourselves,
> automatically, from within some central place in the testsuite (as
> soon as redirection to a logfile has been requested or something)?
> Unless you still have some more magic up your sleeve to handle this,
> Yasuhito :-).
... Ah, I know it is already resolved without using os.dup/dup2
by one of subscriber of this list but not me, and waiting for
contribution :)
Cheers,
--
Yaushito FUTATSUKI <futatuki_at_poem.co.jp>/<futatuki_at_yf.bsdclub.org>
Received on 2020-05-07 13:45:47 CEST