Merge lp:~gocept/landscape-client/py3-fs into lp:~landscape/landscape-client/trunk

[Steffen Allner]

Split off read/create/append_file into text and binary file counterparts to serve strictly one purpose, thus clarifying the intention in each individual call.

Make rest of the code in landscape/lib/fs.py python2/3 compatible.

We are on the journey to get the Bytes / String story solved for Python 2/3 compatibility.

This MP considers code in the landscape.lib.fs module, which handles the reading of files. In Python 3 we can only seek at the end of a file opened in text mode, so the file has to be opened in binary mode. On the other hand, the code using `landscape.lib.fs.read_file()` is expecting a string as result, therefore we decode with the default encoding of 'utf-8'. This could be made configurable, but I did not see any usage in the code, where it might be needed.

Additionally the other methods were updated to use `open()` as context manager.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Fail
Revno: 960
Branch: lp:~gocept/landscape-client/py3-fs
Jenkins: https://ci.lscape.net/job/latch-test-precise/903/

review: Needs Fixing (test results)

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Fail
Revno: 960
Branch: lp:~gocept/landscape-client/py3-fs
Jenkins: https://ci.lscape.net/job/latch-test-xenial/1467/

review: Needs Fixing (test results)

[Данило Шеган (danilo)]

Just a nit about comments/docstrings, looks good.

review: Approve

[lp:~gocept/landscape-client/py3-fs]

961. By Steffen Allner on 2017-03-10

Improve docmentation.

[Steffen Allner (sallner)]

Thanks for the comment. Sometimes I tend to explain to verbose.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Fail
Revno: 961
Branch: lp:~gocept/landscape-client/py3-fs
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3599/

review: Needs Fixing (test results)

[Данило Шеган (danilo)]

The tests don't seem to pass for either python 2 or python 3 with this change.

review: Needs Fixing

[Steffen Allner (sallner)]

> The tests don't seem to pass for either python 2 or python 3 with this change.

I am working on it atm. It is not so clear to me, whether read_file() should return strings or bytes. At most places where it is used, the other components are string literals ("...") in Python 2 but may be piped into structs or similar things. On the other hand, only returning bytes would require a rewrite of all places of usage. I currently think, I would leave the default on bytes and allow optional decoding for Python 3.

[lp:~gocept/landscape-client/py3-fs]

962. By Steffen Allner on 2017-03-10

Do not use context manager due to mock usage.

963. By Steffen Allner on 2017-03-13

Backmerge from trunk.

964. By Steffen Allner on 2017-03-13

Make encoding optional when creating or reading files. This allows to keep the code using strings in most places using Python 2 and 3.

965. By Steffen Allner on 2017-03-13

Use encoding when working with files in order ot get strings in Python 3.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 965
Branch: lp:~gocept/landscape-client/py3-fs
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3604/

review: Approve (test results)

[Данило Шеган (danilo)]

I still think we should really decode/encode unconditionally in read_file/create_file/append_file functions: they should return/take only unicode strings. A more detailed explanation is inline, but I don't think there is a way around this.

I understand that this makes it harder to do things strictly "per module", but once you update a shared helper like this one, all callsites will need to be fixed to deal with unicode too.

Note that our python2 unit tests are blocking, so without them passing, we can't land code. This will make this branch grow a bit, but I don't really see a way around it.

review: Needs Fixing

[Данило Шеган (danilo)]

Actually, it seems create_deb() helper in landscape/package/tests/helpers.py wants to create a binary file, but that is just a test helper, so that's easy to move away from create_file().

However, other callsites are trickier: they do want to construct binary files, and I think we should make a separate helper for them: "create_binary_file()" (mostly those that write out actual .deb files). If needed for tests, we could have a "read_binary_file()" too (which will provide symmetry too).

So my suggestion is to have separate create_binary_file() and create_text_file() helpers with their read_{binary|text}_file counterparts, with text files encoding/decoding from UTF-8, and making append_file be just append_text_file, again using UTF-8 (I doubt there is a need for append_binary_file).

[Steffen Allner (sallner)]

> Actually, it seems create_deb() helper in landscape/package/tests/helpers.py
> wants to create a binary file, but that is just a test helper, so that's easy
> to move away from create_file().
>
> However, other callsites are trickier: they do want to construct binary files,
> and I think we should make a separate helper for them: "create_binary_file()"
> (mostly those that write out actual .deb files). If needed for tests, we
> could have a "read_binary_file()" too (which will provide symmetry too).

Yeah it is always more complicated. That were the failures, the landscape-builder spotted first.

>
> So my suggestion is to have separate create_binary_file() and
> create_text_file() helpers with their read_{binary|text}_file counterparts,
> with text files encoding/decoding from UTF-8, and making append_file be just
> append_text_file, again using UTF-8 (I doubt there is a need for
> append_binary_file).

I am happy with the distinction of binary and text modes for these helper methods. This makes it a clear and readable option directly at the callsite.

This is almost a case, where the introduction of type annotation might be a reasonable thing.

[Данило Шеган (danilo)]

Right, you basically have that, it's just about making it separate methods instead of doing that with the encoding parameter.

[lp:~gocept/landscape-client/py3-fs]

966. By Steffen Allner on 2017-03-13

Explicitly offer a append_{binary|text}_file().

967. By Steffen Allner on 2017-03-13

Provide explicitly file creation in binary or test mode.

968. By Steffen Allner on 2017-03-13

Provide methods for explicitly reading in text or binary mode.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 968
Branch: lp:~gocept/landscape-client/py3-fs
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3610/

review: Approve (test results)

[Steffen Allner (sallner)]

I added separate functions for create, append, and read operations in binary and text mode.

Of course I needed also an append_binary_file() as on of the two modules using it, mangles with TagSection of the apt_pkg module, so there came no easier solution to my mind.

[Данило Шеган (danilo)]

Looks great, thanks for working on this! I just realized the error of my ways (regarding use of codecs module instead of just doing a bytes.decode() and unicode.encode()), but it's minor and not a blocker. +1

review: Approve

[lp:~gocept/landscape-client/py3-fs]

969. By Steffen Allner on 2017-03-14

Directly decode bytes and encode strings.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 969
Branch: lp:~gocept/landscape-client/py3-fs
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3612/

review: Approve (test results)

[Steffen Allner (sallner)]

I changed it back to a direct call and removed the import of codecs. Then this MP should be ready I think. Based on that, I will look at the MP concerning l.l.twisted_util and l.l.network.

[Данило Шеган (danilo)]

Looks good, thanks again! (We need two review passes, I'll ping my colleagues to get on this)

review: Approve

[Данило Шеган (danilo)]

Actually, one Canonical review is sufficient, we need a Gocept review too.

[Daniel Havlik (nilo)]

+1

review: Approve

[Adam Collard (adam-collard)]

review: Approve

[lp:~gocept/landscape-client/py3-fs]

970. By Steffen Allner on 2017-03-14

Trim characters instead of bytes when reading in textmode.

[Steffen Allner (sallner)]

I changed the functionality to trimming characters in text mode.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 970
Branch: lp:~gocept/landscape-client/py3-fs
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3621/

review: Approve (test results)

=== modified file 'landscape/broker/store.py'
--- landscape/broker/store.py	2017-03-06 10:38:07 +0000
+++ landscape/broker/store.py	2017-03-14 14:46:07 +0000
@@ -101,7 +101,7 @@
 
 from landscape import DEFAULT_SERVER_API
 from landscape.compat import bpickle
-from landscape.lib.fs import create_file
+from landscape.lib.fs import create_binary_file
 from landscape.lib.versioning import sort_versions, is_version_higher
 
 
@@ -372,7 +372,7 @@
 
         filename = self._get_next_message_filename()
         temp_path = filename + ".tmp"
-        create_file(temp_path, message_data)
+        create_binary_file(temp_path, message_data)
         os.rename(temp_path, filename)
 
         if not self.accepts(message["type"]):

=== modified file 'landscape/broker/tests/test_store.py'
--- landscape/broker/tests/test_store.py	2016-08-17 20:31:48 +0000
+++ landscape/broker/tests/test_store.py	2017-03-14 14:46:07 +0000
@@ -280,7 +280,7 @@
         self.store.add({"type": "data", "data": 1})
         # We simulate it by creating a fake file which raises halfway through
         # writing a file.
-        with mock.patch("__builtin__.open") as mock_open:
+        with mock.patch("landscape.lib.fs.open") as mock_open:
             mocked_file = mock_open.return_value
             mocked_file.write.side_effect = IOError("Sorry, pal!")
             # This kind of ensures that raising an exception is somewhat
@@ -288,7 +288,7 @@
             # on special exception-handling in the file-writing code.
             self.assertRaises(
                 IOError, self.store.add, {"type": "data", "data": 2})
-            mock_open.assert_called_with(mock.ANY, "w")
+            mock_open.assert_called_with(mock.ANY, "wb")
             mocked_file.write.assert_called_once_with(mock.ANY)
         self.assertEqual(self.store.get_pending_messages(),
                          [{"type": "data", "data": 1, "api": "3.2"}])

=== modified file 'landscape/lib/fs.py'
--- landscape/lib/fs.py	2013-05-28 08:48:31 +0000
+++ landscape/lib/fs.py	2017-03-14 14:46:07 +0000
@@ -1,50 +1,98 @@
 """File-system utils"""
-
 import os
 import time
 
 
-def create_file(path, content):
+from twisted.python.compat import long
+
+
+def create_text_file(path, content):
     """Create a file with the given content.
 
-    @param path: The path to the file.
-    @param content: The content to be written in the file.
-    """
-    fd = open(path, "w")
-    fd.write(content)
-    fd.close()
-
-
-def append_file(path, content):
+    The content is encoded with utf-8 before writing.
+
+    @param path: The path to the file.
+    @param content: The content to be written in the file.
+    """
+    create_binary_file(path, content.encode("utf-8"))
+
+
+def create_binary_file(path, content):
+    """Create a file with the given binary content.
+
+    @param path: The path to the file.
+    @param content: The content to be written in the file.
+    """
+    # XXX: Due to a very specific mock of `open()` in landscape.broker.tests.\
+    # test_store.MessageStoreTest.test_atomic_message_writing it is hard to
+    # write this file opening as context manager.
+    fd = open(path, "wb")
+    try:
+        fd.write(content)
+    finally:
+        fd.close()
+
+
+def append_text_file(path, content):
     """Append a file with the given content.
 
     The file is created, if it doesn't exist already.
 
-    @param path: The path to the file.
-    @param content: The content to be written in the file at the end.
-    """
-    fd = open(path, "a")
-    fd.write(content)
-    fd.close()
-
-
-def read_file(path, limit=None):
-    """Return the content of the given file.
-
-    @param path: The path to the file.
-    @param limit: An optional read limit. If positive, read up to that number
-        of bytes from the beginning of the file. If negative, read up to that
-        number of bytes from the end of the file.
-    @return content: The content of the file, possibly trimmed to C{limit}.
-    """
-    fd = open(path, "r")
-    if limit and os.path.getsize(path) > abs(limit):
-        whence = 0
-        if limit < 0:
-            whence = 2
-        fd.seek(limit, whence)
-    content = fd.read()
-    fd.close()
+    The content is utf-8 encoded before it is written.
+
+    @param path: The path to the file.
+    @param content: The content to be written in the file at the end.
+    """
+    append_binary_file(path, content.encode("utf-8"))
+
+
+def append_binary_file(path, content):
+    """Append a file with the given binary content.
+
+    The file is created, if it doesn't exist already.
+
+    @param path: The path to the file.
+    @param content: The content to be written in the file at the end.
+    """
+    with open(path, "ab") as fd:
+        fd.write(content)
+
+
+def read_text_file(path, limit=None):
+    """Return the content of the given file as string.
+
+    @param path: The path to the file.
+    @param limit: An optional read limit. If positive, read up to that number
+        of bytes from the beginning of the file. If negative, read up to that
+        number of bytes from the end of the file.
+    @return content: The content of the file string, possibly trimmed to
+        C{limit}.
+    """
+    # Use binary mode since opening a file in text mode in Python 3 does not
+    # allow non-zero offset seek from the end of the file.
+    content = read_binary_file(path).decode("utf-8")
+    if limit and len(content) > abs(limit):
+        content = content[limit:]
+    return content
+
+
+def read_binary_file(path, limit=None):
+    """Return the content of the given file as bytes.
+
+    @param path: The path to the file.
+    @param limit: An optional read limit. If positive, read up to that number
+        of bytes from the beginning of the file. If negative, read up to that
+        number of bytes from the end of the file.
+    @return content: The content of the file as bytes, possibly trimmed to
+        C{limit}.
+    """
+    with open(path, "rb") as fd:
+        if limit and os.path.getsize(path) > abs(limit):
+            whence = 0
+            if limit < 0:
+                whence = 2
+            fd.seek(limit, whence)
+        content = fd.read()
     return content
 
 

=== modified file 'landscape/lib/juju.py'
--- landscape/lib/juju.py	2014-09-24 08:09:55 +0000
+++ landscape/lib/juju.py	2017-03-14 14:46:07 +0000
@@ -2,7 +2,7 @@
 import json
 import logging
 
-from landscape.lib.fs import read_file
+from landscape.lib.fs import read_text_file
 
 
 def get_juju_info(config):
@@ -13,7 +13,7 @@
     if not os.path.exists(config.juju_filename):
         return
 
-    json_contents = read_file(config.juju_filename)
+    json_contents = read_text_file(config.juju_filename)
     try:
         juju_info = json.loads(json_contents)
     # Catch any error the json lib could throw, because we don't know or

=== modified file 'landscape/lib/tests/test_fs.py'
--- landscape/lib/tests/test_fs.py	2016-06-15 16:42:17 +0000
+++ landscape/lib/tests/test_fs.py	2017-03-14 14:46:07 +0000
@@ -1,44 +1,90 @@
+# -*- coding: utf-8 -*-
+import codecs
 import os
 from mock import patch
 import time
 
+from twisted.python.compat import long
+
 from landscape.tests.helpers import LandscapeTest
 
-from landscape.lib.fs import append_file, read_file, touch_file
+from landscape.lib.fs import append_text_file, touch_file
+from landscape.lib.fs import read_text_file, read_binary_file
 
 
 class ReadFileTest(LandscapeTest):
 
-    def test_read_file(self):
+    def test_read_binary_file(self):
         """
-        With no options L{read_file} reads the whole file passed as argument.
+        With no options L{read_binary_file} reads the whole file passed as
+        argument.
         """
         path = self.makeFile("foo")
-        self.assertEqual(read_file(path), "foo")
+        self.assertEqual(read_binary_file(path), b"foo")
 
-    def test_read_file_with_limit(self):
+    def test_read_binary_file_with_limit(self):
         """
-        With a positive limit L{read_file} reads only the bytes after the
-        given limit.
+        With a positive limit L{read_binary_file} reads only the bytes after
+        the given limit.
         """
         path = self.makeFile("foo bar")
-        self.assertEqual(read_file(path, limit=3), " bar")
-
-    def test_read_file_with_negative_limit(self):
-        """
-        With a negative limit L{read_file} reads only the tail of the file.
-        """
-        path = self.makeFile("foo bar from end")
-        self.assertEqual(read_file(path, limit=-3), "end")
-
-    def test_read_file_with_limit_bigger_than_file(self):
-        """
-        If the limit is bigger than the file L{read_file} reads the entire
-        file.
-        """
-        path = self.makeFile("foo bar from end")
-        self.assertEqual(read_file(path, limit=100), "foo bar from end")
-        self.assertEqual(read_file(path, limit=-100), "foo bar from end")
+        self.assertEqual(read_binary_file(path, limit=3), b" bar")
+
+    def test_read_binary_file_with_negative_limit(self):
+        """
+        With a negative limit L{read_binary_file} reads only the tail of the
+        file.
+        """
+        path = self.makeFile("foo bar from end")
+        self.assertEqual(read_binary_file(path, limit=-3), b"end")
+
+    def test_read_binary_file_with_limit_bigger_than_file(self):
+        """
+        If the limit is bigger than the file L{read_binary_file} reads the
+        entire file.
+        """
+        path = self.makeFile("foo bar from end")
+        self.assertEqual(
+            read_binary_file(path, limit=100), b"foo bar from end")
+        self.assertEqual(
+            read_binary_file(path, limit=-100), b"foo bar from end")
+
+    def test_read_text_file(self):
+        """
+        With no options L{read_text_file} reads the whole file passed as
+        argument as string decoded with utf-8.
+        """
+        utf8_content = codecs.encode(u"foo \N{SNOWMAN}", "utf-8")
+        path = self.makeFile(utf8_content, mode="wb")
+        self.assertEqual(read_text_file(path), u"foo ☃")
+
+    def test_read_text_file_with_limit(self):
+        """
+        With a positive limit L{read_text_file} returns only the characters
+        after the first L{limit} characters as string.
+        """
+        utf8_content = codecs.encode(u"foo \N{SNOWMAN}", "utf-8")
+        path = self.makeFile(utf8_content, mode="wb")
+        self.assertEqual(read_text_file(path, limit=3), u" ☃")
+
+    def test_read_text_file_with_negative_limit(self):
+        """
+        With a negative limit L{read_text_file} reads only the tail characters
+        of the string.
+        """
+        utf8_content = codecs.encode(u"foo \N{SNOWMAN} bar", "utf-8")
+        path = self.makeFile(utf8_content, mode="wb")
+        self.assertEqual(read_text_file(path, limit=-5), u"☃ bar")
+
+    def test_read_text_file_with_limit_bigger_than_file(self):
+        """
+        If the limit is bigger than the file L{read_text_file} reads the entire
+        file.
+        """
+        utf8_content = codecs.encode(u"foo \N{SNOWMAN} bar", "utf-8")
+        path = self.makeFile(utf8_content, mode="wb")
+        self.assertEqual(read_text_file(path, limit=100), u"foo ☃ bar")
+        self.assertEqual(read_text_file(path, limit=-100), u"foo ☃ bar")
 
 
 class TouchFileTest(LandscapeTest):
@@ -86,19 +132,36 @@
 
 class AppendFileTest(LandscapeTest):
 
-    def test_append_existing_file(self):
-        """
-        The L{append_file} function appends contents to an existing file.
-        """
-        existing_file = self.makeFile("foo bar")
-        append_file(existing_file, " baz")
-        self.assertFileContent(existing_file, "foo bar baz")
-
-    def test_append_no_file(self):
-        """
-        The L{append_file} function creates a new file if one doesn't
-        exist already.
-        """
-        new_file = os.path.join(self.makeDir(), "new_file")
-        append_file(new_file, "contents")
+    def test_append_existing_text_file(self):
+        """
+        The L{append_text_file} function appends contents to an existing file.
+        """
+        existing_file = self.makeFile("foo bar")
+        append_text_file(existing_file, " baz")
+        self.assertFileContent(existing_file, "foo bar baz")
+
+    def test_append_text_no_file(self):
+        """
+        The L{append_text_file} function creates a new file if one doesn't
+        exist already.
+        """
+        new_file = os.path.join(self.makeDir(), "new_file")
+        append_text_file(new_file, "contents")
+        self.assertFileContent(new_file, "contents")
+
+    def test_append_existing_binary_file(self):
+        """
+        The L{append_text_file} function appends contents to an existing file.
+        """
+        existing_file = self.makeFile("foo bar")
+        append_text_file(existing_file, " baz")
+        self.assertFileContent(existing_file, "foo bar baz")
+
+    def test_append_binary_no_file(self):
+        """
+        The L{append_text_file} function creates a new file if one doesn't
+        exist already.
+        """
+        new_file = os.path.join(self.makeDir(), "new_file")
+        append_text_file(new_file, "contents")
         self.assertFileContent(new_file, "contents")

=== modified file 'landscape/lib/tests/test_process.py'
--- landscape/lib/tests/test_process.py	2017-03-09 10:13:30 +0000
+++ landscape/lib/tests/test_process.py	2017-03-14 14:46:07 +0000
@@ -5,7 +5,7 @@
 from landscape.tests.helpers import LandscapeTest
 
 from landscape.lib.process import calculate_pcpu, ProcessInformation
-from landscape.lib.fs import create_file
+from landscape.lib.fs import create_text_file
 
 
 class ProcessInfoTest(LandscapeTest):
@@ -25,7 +25,7 @@
         os.mkdir(process_dir)
 
         cmd_line = "/usr/bin/foo"
-        create_file(os.path.join(process_dir, "cmdline"), cmd_line)
+        create_text_file(os.path.join(process_dir, "cmdline"), cmd_line)
 
         status = "\n".join([
             "Name: foo",
@@ -34,11 +34,11 @@
             "Gid: 2000",
             "VmSize: 3000",
             "Ignored: value"])
-        create_file(os.path.join(process_dir, "status"), status)
+        create_text_file(os.path.join(process_dir, "status"), status)
 
         stat_array = [str(index) for index in range(44)]
         stat = " ".join(stat_array)
-        create_file(os.path.join(process_dir, "stat"), stat)
+        create_text_file(os.path.join(process_dir, "stat"), stat)
 
     @mock.patch("landscape.lib.process.detect_jiffies", return_value=1)
     @mock.patch("os.listdir")

=== modified file 'landscape/lib/tests/test_twisted_util.py'
--- landscape/lib/tests/test_twisted_util.py	2017-01-23 12:14:33 +0000
+++ landscape/lib/tests/test_twisted_util.py	2017-03-14 14:46:07 +0000
@@ -3,7 +3,7 @@
 from twisted.python.compat import _PY3
 from unittest import skipIf
 
-from landscape.lib.fs import create_file
+from landscape.lib.fs import create_text_file
 from landscape.lib.twisted_util import spawn_process
 from landscape.tests.helpers import LandscapeTest
 
@@ -19,7 +19,7 @@
         """
         The process is executed and returns the expected exit code.
         """
-        create_file(self.command, "#!/bin/sh\nexit 2")
+        create_text_file(self.command, "#!/bin/sh\nexit 2")
 
         def callback(args):
             out, err, code = args
@@ -49,7 +49,7 @@
         """
         The process returns the expected standard error.
         """
-        create_file(self.command, "#!/bin/sh\necho -n $@ >&2")
+        create_text_file(self.command, "#!/bin/sh\necho -n $@ >&2")
 
         def callback(args):
             out, err, code = args
@@ -66,7 +66,7 @@
         If a callback for process output is provieded, it is called for every
         line of output.
         """
-        create_file(self.command, "#!/bin/sh\n/bin/echo -ne $@")
+        create_text_file(self.command, "#!/bin/sh\n/bin/echo -ne $@")
         param = r"some text\nanother line\nok, last one\n"
         expected = ["some text", "another line", "ok, last one"]
         lines = []
@@ -87,7 +87,7 @@
         """
         If output ends with more than one newline, empty lines are preserved.
         """
-        create_file(self.command, "#!/bin/sh\n/bin/echo -ne $@")
+        create_text_file(self.command, "#!/bin/sh\n/bin/echo -ne $@")
         param = r"some text\nanother line\n\n\n"
         expected = ["some text", "another line", "", ""]
         lines = []
@@ -109,7 +109,7 @@
         If output ends without a newline, the line is still passed to the
         callback.
         """
-        create_file(self.command, "#!/bin/sh\n/bin/echo -ne $@")
+        create_text_file(self.command, "#!/bin/sh\n/bin/echo -ne $@")
         param = r"some text\nanother line\nok, last one"
         expected = ["some text", "another line", "ok, last one"]
         lines = []
@@ -131,7 +131,7 @@
         """
         Optionally C{spawn_process} accepts a C{stdin} argument.
         """
-        create_file(self.command, "#!/bin/sh\n/bin/cat")
+        create_text_file(self.command, "#!/bin/sh\n/bin/cat")
 
         def callback(args):
             out, err, code = args

=== modified file 'landscape/lib/vm_info.py'
--- landscape/lib/vm_info.py	2016-09-23 15:43:36 +0000
+++ landscape/lib/vm_info.py	2017-03-14 14:46:07 +0000
@@ -3,7 +3,7 @@
 """
 import os
 
-from landscape.lib.fs import read_file
+from landscape.lib.fs import read_text_file
 
 
 def get_vm_info(root_path="/"):
@@ -34,7 +34,7 @@
     for filename in ("container_type", "systemd/container"):
         path = os.path.join(run_path, filename)
         if os.path.exists(path):
-            return read_file(path).strip()
+            return read_text_file(path).strip()
     return ""
 
 
@@ -52,7 +52,7 @@
 
 def _get_vm_by_vendor(sys_vendor_path):
     """Return the VM type string (possibly empty) based on the vendor."""
-    vendor = read_file(sys_vendor_path).lower()
+    vendor = read_text_file(sys_vendor_path).lower()
     # Use lower-key string for vendors, since we do case-insentive match.
     content_vendors_map = (
         ("bochs", "kvm"),
@@ -72,7 +72,7 @@
 def _get_vm_legacy(root_path):
     """Check if the host is virtualized looking at /proc/cpuinfo content."""
     try:
-        cpuinfo = read_file(os.path.join(root_path, "proc/cpuinfo"))
+        cpuinfo = read_text_file(os.path.join(root_path, "proc/cpuinfo"))
     except (IOError, OSError):
         return ""
 

=== modified file 'landscape/monitor/aptpreferences.py'
--- landscape/monitor/aptpreferences.py	2017-01-10 09:43:40 +0000
+++ landscape/monitor/aptpreferences.py	2017-03-14 14:46:07 +0000
@@ -1,8 +1,8 @@
 import os
 
-from twisted.python.compat import iteritems, unicode
+from twisted.python.compat import iteritems
 
-from landscape.lib.fs import read_file
+from landscape.lib.fs import read_text_file
 from landscape.constants import APT_PREFERENCES_SIZE_LIMIT
 
 from landscape.monitor.plugin import DataWatcher
@@ -30,11 +30,10 @@
         simply return C{None}
         """
         data = {}
-        read_unicode = lambda filename: unicode(read_file(filename))
         preferences_filename = os.path.join(self._etc_apt_directory,
                                             u"preferences")
         if os.path.exists(preferences_filename):
-            data[preferences_filename] = read_unicode(preferences_filename)
+            data[preferences_filename] = read_text_file(preferences_filename)
 
         preferences_directory = os.path.join(self._etc_apt_directory,
                                              u"preferences.d")
@@ -42,7 +41,7 @@
             for entry in os.listdir(preferences_directory):
                 filename = os.path.join(preferences_directory, entry)
                 if os.path.isfile(filename):
-                    data[filename] = read_unicode(filename)
+                    data[filename] = read_text_file(filename)
 
         if data == {}:
             return None

=== modified file 'landscape/monitor/computerinfo.py'
--- landscape/monitor/computerinfo.py	2013-10-02 15:26:21 +0000
+++ landscape/monitor/computerinfo.py	2017-03-14 14:46:07 +0000
@@ -3,7 +3,7 @@
 from twisted.internet.defer import inlineCallbacks, returnValue
 
 from landscape.lib.fetch import fetch_async
-from landscape.lib.fs import read_file
+from landscape.lib.fs import read_text_file
 from landscape.lib.lsb_release import LSB_RELEASE_FILENAME, parse_lsb_release
 from landscape.lib.cloud import fetch_ec2_meta_data
 from landscape.lib.network import get_fqdn
@@ -89,7 +89,7 @@
         annotations = {}
         if os.path.exists(self._annotations_path):
             for key in os.listdir(self._annotations_path):
-                annotations[key] = read_file(
+                annotations[key] = read_text_file(
                     os.path.join(self._annotations_path, key))
 
         if annotations:

=== modified file 'landscape/monitor/rebootrequired.py'
--- landscape/monitor/rebootrequired.py	2017-03-10 12:14:46 +0000
+++ landscape/monitor/rebootrequired.py	2017-03-14 14:46:07 +0000
@@ -1,7 +1,7 @@
 import os
 import logging
 
-from landscape.lib.fs import read_file
+from landscape.lib.fs import read_text_file
 from landscape.monitor.plugin import MonitorPlugin
 
 
@@ -34,7 +34,7 @@
         if not os.path.exists(self._packages_filename):
             return []
 
-        lines = read_file(self._packages_filename).splitlines()
+        lines = read_text_file(self._packages_filename).splitlines()
         packages = set(line.strip() for line in lines if line)
         return sorted(packages)
 

=== modified file 'landscape/monitor/tests/test_computerinfo.py'
--- landscape/monitor/tests/test_computerinfo.py	2016-06-15 20:15:44 +0000
+++ landscape/monitor/tests/test_computerinfo.py	2017-03-14 14:46:07 +0000
@@ -5,7 +5,7 @@
 from twisted.internet.defer import succeed, fail, inlineCallbacks
 
 from landscape.lib.fetch import HTTPCodeError, PyCurlError
-from landscape.lib.fs import create_file
+from landscape.lib.fs import create_text_file
 from landscape.monitor.computerinfo import ComputerInfo, METADATA_RETRY_MAX
 from landscape.tests.helpers import LandscapeTest, MonitorHelper
 
@@ -367,8 +367,10 @@
         """
         annotations_dir = self.monitor.config.annotations_path
         os.mkdir(annotations_dir)
-        create_file(os.path.join(annotations_dir, "annotation1"), "value1")
-        create_file(os.path.join(annotations_dir, "annotation2"), "value2")
+        create_text_file(
+            os.path.join(annotations_dir, "annotation1"), "value1")
+        create_text_file(
+            os.path.join(annotations_dir, "annotation2"), "value2")
         self.mstore.set_accepted_types(["computer-info"])
 
         plugin = ComputerInfo()

=== modified file 'landscape/package/changer.py'
--- landscape/package/changer.py	2017-01-05 09:07:23 +0000
+++ landscape/package/changer.py	2017-03-14 14:46:07 +0000
@@ -14,7 +14,7 @@
     POLICY_STRICT, POLICY_ALLOW_INSTALLS, POLICY_ALLOW_ALL_CHANGES,
     UNKNOWN_PACKAGE_DATA_TIMEOUT)
 
-from landscape.lib.fs import create_file
+from landscape.lib.fs import create_binary_file
 from landscape.lib.log import log_failure
 from landscape.package.reporter import find_reporter_command
 from landscape.package.taskhandler import (
@@ -172,8 +172,8 @@
         if binaries:
             hash_ids = {}
             for hash, id, deb in binaries:
-                create_file(os.path.join(binaries_path, "%d.deb" % id),
-                            base64.decodestring(deb))
+                create_binary_file(os.path.join(binaries_path, "%d.deb" % id),
+                                   base64.decodestring(deb))
                 hash_ids[hash] = id
             self._store.set_hash_ids(hash_ids)
             self._facade.add_channel_deb_dir(binaries_path)

=== modified file 'landscape/package/facade.py'
--- landscape/package/facade.py	2017-01-23 12:14:33 +0000
+++ landscape/package/facade.py	2017-03-14 14:46:07 +0000
@@ -25,7 +25,8 @@
 
 
 from landscape.compat import StringIO
-from landscape.lib.fs import append_file, create_file, read_file, touch_file
+from landscape.lib.fs import append_text_file, create_text_file
+from landscape.lib.fs import read_text_file, read_binary_file, touch_file
 from landscape.package.skeleton import build_skeleton_apt
 
 
@@ -154,10 +155,10 @@
         self._ensure_sub_dir("var/lib/dpkg/info")
         self._ensure_sub_dir("var/lib/dpkg/updates")
         self._ensure_sub_dir("var/lib/dpkg/triggers")
-        create_file(os.path.join(dpkg_dir, "available"), "")
+        create_text_file(os.path.join(dpkg_dir, "available"), "")
         self._dpkg_status = os.path.join(dpkg_dir, "status")
         if not os.path.exists(self._dpkg_status):
-            create_file(self._dpkg_status, "")
+            create_text_file(self._dpkg_status, "")
         # Apt will fail if it does not have a keyring.  It does not care if
         # the keyring is empty.
         touch_file(os.path.join(apt_dir, "trusted.gpg"))
@@ -279,11 +280,11 @@
         if components:
             sources_line += " %s" % " ".join(components)
         if os.path.exists(sources_file_path):
-            current_content = read_file(sources_file_path).split("\n")
+            current_content = read_text_file(sources_file_path).split("\n")
             if sources_line in current_content:
                 return
         sources_line += "\n"
-        append_file(sources_file_path, sources_line)
+        append_text_file(sources_file_path, sources_line)
 
     def add_channel_deb_dir(self, path):
         """Add a directory with packages as a channel.
@@ -312,7 +313,7 @@
         packages_contents = "\n".join(
             self.get_package_stanza(os.path.join(deb_dir, filename))
             for filename in sorted(os.listdir(deb_dir)))
-        create_file(os.path.join(deb_dir, "Packages"), packages_contents)
+        create_text_file(os.path.join(deb_dir, "Packages"), packages_contents)
 
     def get_channels(self):
         """Return a list of channels configured.
@@ -344,7 +345,7 @@
         deb_file.close()
         filename = os.path.basename(deb_path)
         size = os.path.getsize(deb_path)
-        contents = read_file(deb_path)
+        contents = read_binary_file(deb_path)
         md5 = hashlib.md5(contents).hexdigest()
         sha1 = hashlib.sha1(contents).hexdigest()
         sha256 = hashlib.sha256(contents).hexdigest()
@@ -667,7 +668,7 @@
                         raise SystemError("dpkg didn't exit cleanly.")
                 except (apt.cache.LockFailedException, SystemError) as exception:
                     result_text = (fetch_output.getvalue()
-                                   + read_file(install_output_path))
+                                   + read_text_file(install_output_path))
                     error = TransactionError(exception.args[0] +
                                              "\n\nPackage operation log:\n" +
                                              result_text)
@@ -677,7 +678,7 @@
                         break
                 else:
                     result_text = (fetch_output.getvalue()
-                                   + read_file(install_output_path))
+                                   + read_text_file(install_output_path))
                     break
             if error is not None:
                 raise error

=== modified file 'landscape/package/releaseupgrader.py'
--- landscape/package/releaseupgrader.py	2017-01-23 12:14:33 +0000
+++ landscape/package/releaseupgrader.py	2017-03-14 14:46:07 +0000
@@ -12,7 +12,7 @@
 from landscape.lib.fetch import url_to_filename, fetch_to_files
 from landscape.lib.lsb_release import parse_lsb_release, LSB_RELEASE_FILENAME
 from landscape.lib.gpg import gpg_verify
-from landscape.lib.fs import read_file
+from landscape.lib.fs import read_text_file
 from landscape.package.taskhandler import (
     PackageTaskHandlerConfiguration, PackageTaskHandler, run_task_handler)
 from landscape.lib.twisted_util import spawn_process
@@ -196,7 +196,7 @@
             if not basename.endswith(".log"):
                 continue
             filename = os.path.join(self.logs_directory, basename)
-            content = read_file(filename, - self.logs_limit)
+            content = read_text_file(filename, - self.logs_limit)
             buf.write("=== %s ===\n\n%s\n\n" % (basename, content))
 
         return buf.getvalue()

=== modified file 'landscape/package/tests/helpers.py'
--- landscape/package/tests/helpers.py	2017-01-19 08:34:52 +0000
+++ landscape/package/tests/helpers.py	2017-03-14 14:46:07 +0000
@@ -6,7 +6,8 @@
 import apt_inst
 import apt_pkg
 
-from landscape.lib.fs import append_file, create_file
+from landscape.lib.fs import append_binary_file, append_text_file
+from landscape.lib.fs import create_binary_file
 from landscape.package.facade import AptFacade
 
 
@@ -52,7 +53,7 @@
         package_stanza = apt_pkg.rewrite_section(
             apt_pkg.TagSection(package_stanza), apt_pkg.REWRITE_PACKAGE_ORDER,
             control_fields.items())
-        append_file(packages_file, "\n" + package_stanza + "\n")
+        append_binary_file(packages_file, "\n" + package_stanza + "\n")
 
     def _add_system_package(self, name, architecture="all", version="1.0",
                             control_fields=None):
@@ -73,7 +74,7 @@
         lines = control.splitlines()
         lines.insert(1, "Status: install ok installed")
         status = "\n".join(lines)
-        append_file(self.dpkg_status, status + "\n\n")
+        append_text_file(self.dpkg_status, status + "\n\n")
 
     def _add_package_to_deb_dir(self, path, name, architecture="all",
                                 version="1.0", description="description",
@@ -319,7 +320,7 @@
     """Create a Debian package in the specified C{target_dir}."""
     path = os.path.join(target_dir, pkg_name)
     data = base64.decodestring(pkg_data)
-    create_file(path, data)
+    create_binary_file(path, data)
 
 
 def create_simple_repository(target_dir):

=== modified file 'landscape/package/tests/test_changer.py'
--- landscape/package/tests/test_changer.py	2016-08-17 20:31:48 +0000
+++ landscape/package/tests/test_changer.py	2017-03-14 14:46:07 +0000
@@ -10,7 +10,7 @@
 
 from mock import patch, Mock, call
 
-from landscape.lib.fs import create_file, read_file, touch_file
+from landscape.lib.fs import create_text_file, read_text_file, touch_file
 from landscape.package.changer import (
     PackageChanger, main, find_changer_command, UNKNOWN_PACKAGE_DATA_TIMEOUT,
     SUCCESS_RESULT, DEPENDENCY_ERROR_RESULT, POLICY_ALLOW_INSTALLS,
@@ -101,11 +101,11 @@
     def remove_pkg2(self):
         """Remove package name2 from its repository."""
         packages_file = os.path.join(self.repository_dir, "Packages")
-        packages_contents = read_file(packages_file)
+        packages_contents = read_text_file(packages_file)
         packages_contents = "\n\n".join(
             [stanza for stanza in packages_contents.split("\n\n")
              if "Package: name2" not in stanza])
-        create_file(packages_file, packages_contents)
+        create_text_file(packages_file, packages_contents)
 
     def get_binaries_channels(self, binaries_path):
         """Return the channels that will be used for the binaries."""

=== modified file 'landscape/package/tests/test_facade.py'
--- landscape/package/tests/test_facade.py	2017-01-10 09:43:40 +0000
+++ landscape/package/tests/test_facade.py	2017-03-14 14:46:07 +0000
@@ -11,7 +11,7 @@
 
 from twisted.python.compat import unicode
 
-from landscape.lib.fs import read_file, create_file
+from landscape.lib.fs import read_text_file, create_text_file
 from landscape.package.facade import (
     TransactionError, DependencyError, ChannelError, AptFacade,
     LandscapeInstallProgress)
@@ -183,7 +183,7 @@
         list_filename = (
             self.apt_root +
             "/etc/apt/sources.list.d/_landscape-internal-facade.list")
-        sources_contents = read_file(list_filename)
+        sources_contents = read_text_file(list_filename)
         self.assertEqual(
             "deb http://example.com/ubuntu lucid\n",
             sources_contents)
@@ -199,7 +199,7 @@
         list_filename = (
             self.apt_root +
             "/etc/apt/sources.list.d/_landscape-internal-facade.list")
-        sources_contents = read_file(list_filename)
+        sources_contents = read_text_file(list_filename)
         self.assertEqual(
             "deb http://example.com/ubuntu lucid\n",
             sources_contents)
@@ -216,7 +216,7 @@
         list_filename = (
             self.apt_root +
             "/etc/apt/sources.list.d/_landscape-internal-facade.list")
-        sources_contents = read_file(list_filename)
+        sources_contents = read_text_file(list_filename)
         self.assertEqual(
             "deb http://example.com/ubuntu lucid main restricted\n",
             sources_contents)
@@ -265,9 +265,9 @@
         sources_list_file = apt_pkg.config.find_file("Dir::Etc::sourcelist")
         sources_list_d_file = os.path.join(
             apt_pkg.config.find_dir("Dir::Etc::sourceparts"), "example.list")
-        create_file(
+        create_text_file(
             sources_list_file, "deb http://example1.com/ubuntu lucid main")
-        create_file(
+        create_text_file(
             sources_list_d_file, "deb http://example2.com/ubuntu lucid main")
 
         self.facade.clear_channels()
@@ -321,7 +321,7 @@
         deb_dir = self.makeDir()
         create_simple_repository(deb_dir)
         self.facade.add_channel_deb_dir(deb_dir)
-        packages_contents = read_file(os.path.join(deb_dir, "Packages"))
+        packages_contents = read_text_file(os.path.join(deb_dir, "Packages"))
         expected_contents = "\n".join(
             self.facade.get_package_stanza(os.path.join(deb_dir, pkg_name))
             for pkg_name in [PKGNAME1, PKGNAME2, PKGNAME3])

=== modified file 'landscape/package/tests/test_reporter.py'
--- landscape/package/tests/test_reporter.py	2017-03-06 10:38:07 +0000
+++ landscape/package/tests/test_reporter.py	2017-03-14 14:46:07 +0000
@@ -8,7 +8,7 @@
 from twisted.internet import reactor
 
 
-from landscape.lib.fs import create_file, touch_file
+from landscape.lib.fs import create_text_file, touch_file
 from landscape.lib.fetch import FetchError
 from landscape.package.store import (
     PackageStore, UnknownHashIDRequest, FakePackageStore)
@@ -66,7 +66,7 @@
 
     def _clear_repository(self):
         """Remove all packages from self.repository."""
-        create_file(self.repository_dir + "/Packages", "")
+        create_text_file(self.repository_dir + "/Packages", "")
 
     def set_pkg1_upgradable(self):
         """Make it so that package "name1" is considered to be upgradable.

=== modified file 'landscape/sysinfo/tests/test_deployment.py'
--- landscape/sysinfo/tests/test_deployment.py	2016-06-16 17:00:28 +0000
+++ landscape/sysinfo/tests/test_deployment.py	2017-03-14 14:46:07 +0000
@@ -6,7 +6,7 @@
 
 from twisted.internet.defer import Deferred
 
-from landscape.lib.fs import create_file
+from landscape.lib.fs import create_text_file
 
 from landscape.sysinfo.deployment import (
     SysInfoConfiguration, ALL_PLUGINS, run, setup_logging,
@@ -51,7 +51,7 @@
 
     def test_config_file(self):
         filename = self.makeFile()
-        create_file(filename, "[sysinfo]\nsysinfo_plugins = TestPlugin\n")
+        create_text_file(filename, "[sysinfo]\nsysinfo_plugins = TestPlugin\n")
         self.configuration.load(["--config", filename, "-d", self.makeDir()])
         plugins = self.configuration.get_plugins()
         self.assertEqual(len(plugins), 1)

=== modified file 'landscape/tests/test_deployment.py'
--- landscape/tests/test_deployment.py	2017-01-23 12:14:33 +0000
+++ landscape/tests/test_deployment.py	2017-03-14 14:46:07 +0000
@@ -4,7 +4,7 @@
 import os
 
 from landscape.compat import StringIO
-from landscape.lib.fs import read_file, create_file
+from landscape.lib.fs import read_text_file, create_text_file
 
 from landscape.deployment import (
     BaseConfiguration, Configuration, get_versioned_persist)
@@ -191,7 +191,7 @@
         self.write_config_file(log_level="debug")
         self.config.log_level = "warning"
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]\nlog_level = warning")
 
     def test_write_configuration_with_section(self):
@@ -199,7 +199,7 @@
         self.write_config_file(section_name="babble", whatever="yay")
         self.config.whatever = "boo"
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[babble]\nwhatever = boo")
 
     def test_write_unrelated_configuration_back(self):
@@ -214,7 +214,7 @@
         self.config.load_configuration_file(config_filename)
         self.config.whatever = "boo"
         self.config.write()
-        data = read_file(config_filename)
+        data = read_text_file(config_filename)
         self.assertConfigEqual(
             data,
             "[babble]\nwhatever = boo\n\n[goojy]\nunrelated = yes")
@@ -226,7 +226,7 @@
         self.config.load([])
         self.config.log_level = "warning"
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]\nlog_level = warning\n")
 
     def test_write_empty_list_values_instead_of_double_quotes(self):
@@ -239,7 +239,7 @@
         self.config.load([])
         self.config.include_manager_plugins = ""
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]\ninclude_manager_plugins = \n")
 
     def test_dont_write_config_specified_default_options(self):
@@ -250,7 +250,7 @@
         self.write_config_file(log_level="debug")
         self.config.log_level = "info"
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]")
 
     def test_dont_write_unspecified_default_options(self):
@@ -261,7 +261,7 @@
         self.write_config_file()
         self.config.log_level = "info"
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]")
 
     def test_dont_write_client_section_default_options(self):
@@ -272,7 +272,7 @@
         self.write_config_file(log_level="debug")
         self.config.log_level = "info"
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]")
 
     def test_do_write_preexisting_default_options(self):
@@ -285,7 +285,7 @@
         self.config.load_configuration_file(config_filename)
         self.config.log_level = "info"
         self.config.write()
-        data = read_file(config_filename)
+        data = read_text_file(config_filename)
         self.assertConfigEqual(data, "[client]\nlog_level = info\n")
 
     def test_dont_delete_explicitly_set_default_options(self):
@@ -296,21 +296,21 @@
         """
         self.write_config_file(log_level="info")
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]\nlog_level = info")
 
     def test_dont_write_config_option(self):
         self.write_config_file()
         self.config.config = self.config_filename
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]")
 
     def test_write_command_line_options(self):
         self.write_config_file()
         self.config.load(["--log-level", "warning"])
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]\nlog_level = warning\n")
 
     def test_write_command_line_precedence(self):
@@ -319,7 +319,7 @@
         self.write_config_file(log_level="debug")
         self.config.load(["--log-level", "warning"])
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]\nlog_level = warning\n")
 
     def test_write_manually_set_precedence(self):
@@ -329,7 +329,7 @@
         self.config.load(["--log-level", "warning"])
         self.config.log_level = "error"
         self.config.write()
-        data = read_file(self.config_filename)
+        data = read_text_file(self.config_filename)
         self.assertConfigEqual(data, "[client]\nlog_level = error\n")
 
     def test_write_to_given_config_file(self):
@@ -338,7 +338,7 @@
             ["--log-level", "warning", "--config", filename])
         self.config.log_level = "error"
         self.config.write()
-        data = read_file(filename)
+        data = read_text_file(filename)
         self.assertConfigEqual(data, "[client]\nlog_level = error\n")
 
     def test_comments_are_maintained(self):
@@ -351,7 +351,7 @@
         self.config.load_configuration_file(filename)
         self.config.log_level = "error"
         self.config.write()
-        new_config = read_file(filename)
+        new_config = read_text_file(filename)
         self.assertConfigEqual(
             new_config,
             "[client]\n# Comment 1\nlog_level = error\n#Comment 2\n")
@@ -412,7 +412,7 @@
         """
         filename = self.makeFile("[client]\nhello = world1\n")
         self.config.load(["--config", filename])
-        create_file(filename, "[client]\nhello = world2\n")
+        create_text_file(filename, "[client]\nhello = world2\n")
         self.config.reload()
         self.assertEqual(self.config.hello, "world2")