Branko Čibej wrote:
> On 12.06.2018 13:00, Julian Foad wrote:
>> Branko Čibej wrote:
>>> https://ci.apache.org/builders/svn-x64-macosx-apr1.3-nothread/builds/3416/steps/Test%20ra_local%2Bfsfs/logs/faillog
>>>
>> I should have a chance to debug this on a Mac today or tomorrow.
> 
> The good news is that I can't reproduce this test failure locally. There
> are two possible reasons I can think of: [...]

I have been trying to reproduce and debug this with my colleague Konrad and we 
have a theory. The failure mode seems consistent with Python writing a change 
to disk and then running 'svn', and 'svn' not seeing that change on disk.

The test uses this code in svntest.main.file_write():

  open(path, mode).write(contents)

When opening and using a file object "in line" in this way, is seems that the 
timing of the implicit "close" is not specified, according to sources such as:

  
https://stackoverflow.com/questions/5362901/in-the-inline-open-and-write-file-is-the-close-implicit#5362917

"""
The close() here happens when the file object is deallocated from memory, as 
part of its deletion logic. Because modern Pythons on other virtual machines — 
like Java and .NET — cannot control when an object is deallocated from memory, 
it is no longer considered good Python to open() like this without a close(). 
The recommendation today is to use a with statement, which explicitly requests 
a close() when the block is exited ...
"""

It seems that we need to change the code to something like:

def file_write(path, contents, mode='w'):
  [...]
  with open(path, mode) as f:
    f.write(contents)

to guarantee the file is closed before the test proceeds further. It may be 
that most of our testing in the past has used Python implementations where the 
write is never delayed far enough to affect what an invoked 'svn' subprocess 
sees, but on the Mac it is a little different. We are using a Mac with Python 
2. (The 'file_write' method has separate code paths for Python 2 and Python 3 
but both use the same open().write() construct.)

The same construct appears in many other places in the test suite too.

We are trying to test this, and it is taking a long time because we can only 
currently reproduce the failure after a fresh checkout/build/test cycle. Maybe 
because the bug is sensitive to the differences in timing of a warm cache vs. a 
cold cache.

- Julian

Reply via email to