Re: shelf-tests failure on macOS

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
Received on 2018-06-13 16:31:17 CEST