diff --git a/osc/commandline.py b/osc/commandline.py index c1c042b0..5bd5a7b3 100644 --- a/osc/commandline.py +++ b/osc/commandline.py @@ -4249,13 +4249,10 @@ Please submit there instead, or use --nodevelproject to force direct submission. def _prdiff_output_diff(self, opts, rdiff): if opts.diffstat: print() - p = subprocess.Popen("diffstat", - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - close_fds=True) - p.stdin.write(rdiff) - p.stdin.close() - print("".join(decode_it(x) for x in p.stdout.readlines())) + with subprocess.Popen("diffstat", stdin=subprocess.PIPE, stdout=subprocess.PIPE, close_fds=True) as p: + p.stdin.write(rdiff) + p.stdin.close() + print("".join(decode_it(x) for x in p.stdout.readlines())) elif opts.unified: print() if isinstance(rdiff, str): diff --git a/osc/connection.py b/osc/connection.py index 0154c0f8..db7afd64 100644 --- a/osc/connection.py +++ b/osc/connection.py @@ -123,6 +123,24 @@ HTTP_PROXY_MANAGER = get_proxy_manager("HTTP_PROXY") HTTPS_PROXY_MANAGER = get_proxy_manager("HTTPS_PROXY") +def http_request_wrap_file(func): + """ + Turn file path into a file object and close it automatically + by using a context manager. + """ + def new_func(method, url, headers=None, data=None, file=None): + if file: + with open(file, "rb") as f: + return func(method, url, headers, data, file=f) + else: + return func(method, url, headers, data, file) + + new_func.__name__ = func.__name__ + new_func.__doc__ = func.__doc__ + return new_func + + +@http_request_wrap_file def http_request(method, url, headers=None, data=None, file=None): """ Send a HTTP request to a server. @@ -162,8 +180,8 @@ def http_request(method, url, headers=None, data=None, file=None): data = data.encode("utf-8") content_length = len(data) elif file: - content_length = os.path.getsize(file) - data = open(file, "rb") + content_length = os.fstat(file.fileno()).st_size + data = file else: content_length = 0 diff --git a/osc/core.py b/osc/core.py index 3716281e..f039315f 100644 --- a/osc/core.py +++ b/osc/core.py @@ -2023,7 +2023,8 @@ class Package: if not add: tmpl = b'-%s' ltmpl = b'@@ -1,%d +0,0 @@\n' - lines = [tmpl % i for i in open(fname, 'rb').readlines()] + with open(fname, 'rb') as f: + lines = [tmpl % i for i in f.readlines()] if len(lines): diff.append(ltmpl % len(lines)) if not lines[-1].endswith(b'\n'): @@ -3277,7 +3278,8 @@ def store_readlist(dir, name): r = [] if os.path.exists(os.path.join(dir, store, name)): - r = [line.rstrip('\n') for line in open(os.path.join(dir, store, name))] + with open(os.path.join(dir, store, name)) as f: + r = [line.rstrip('\n') for line in f] return r def read_tobeadded(dir): @@ -3293,7 +3295,8 @@ def read_sizelimit(dir): fname = os.path.join(dir, store, '_size_limit') if os.path.exists(fname): - r = open(fname).readline().strip() + with open(fname) as f: + r = f.readline().strip() if r is None or not r.isdigit(): return None @@ -3351,7 +3354,8 @@ def check_store_version(dir): versionfile = os.path.join(dir, store, '_osclib_version') try: - v = open(versionfile).read().strip() + with open(versionfile) as f: + v = f.read().strip() except: v = '' @@ -4741,7 +4745,8 @@ def binary(s): def binary_file(fn): """read 4096 bytes from a file named fn, and call binary() on the data""" - return binary(open(fn, 'rb').read(4096)) + with open(fn, 'rb') as f: + return binary(f.read(4096)) def get_source_file_diff(dir, filename, rev, oldfilename = None, olddir = None, origfilename = None): @@ -6500,7 +6505,8 @@ def store_read_project(dir): global store try: - p = open(os.path.join(dir, store, '_project')).readlines()[0].strip() + with open(os.path.join(dir, store, '_project')) as f: + p = f.readline().strip() except OSError: msg = 'Error: \'%s\' is not an osc project dir or working copy' % os.path.abspath(dir) if os.path.exists(os.path.join(dir, '.svn')): @@ -6513,7 +6519,8 @@ def store_read_package(dir): global store try: - p = open(os.path.join(dir, store, '_package')).readlines()[0].strip() + with open(os.path.join(dir, store, '_package')) as f: + p = f.readline().strip() except OSError: msg = 'Error: \'%s\' is not an osc package working copy' % os.path.abspath(dir) if os.path.exists(os.path.join(dir, '.svn')): @@ -6541,7 +6548,8 @@ def store_read_apiurl(dir, defaulturl=True): fname = os.path.join(dir, store, '_apiurl') try: - url = open(fname).readlines()[0].strip() + with open(fname) as f: + url = f.readlines()[0].strip() # this is needed to get a proper apiurl # (former osc versions may stored an apiurl with a trailing slash etc.) apiurl = conf.urljoin(*conf.parse_apisrv_url(None, url)) @@ -6612,8 +6620,8 @@ def store_read_file(dir, file): global store try: - content = open(os.path.join(dir, store, file)).read() - return content + with open(os.path.join(dir, store, file)) as f: + return f.read() except: return None diff --git a/tests/common.py b/tests/common.py index 0cb5c200..0e6766b8 100644 --- a/tests/common.py +++ b/tests/common.py @@ -233,8 +233,7 @@ class OscTestCase(unittest.TestCase): def _check_list(self, fname, exp): fname = os.path.join('.osc', fname) - self.assertTrue(os.path.exists(fname)) - self.assertEqual(open(fname).read(), exp) + self.assertFileContentEqual(fname, exp) def _check_addlist(self, exp): self._check_list('_to_be_added', exp) @@ -262,6 +261,22 @@ class OscTestCase(unittest.TestCase): self.assertTrue(os.path.exists(os.path.join('.osc', i.get('name')))) self.assertEqual(osc.core.dgst(os.path.join('.osc', i.get('name'))), i.get('md5')) + def assertFilesEqual(self, first, second): + self.assertTrue(os.path.exists(first)) + self.assertTrue(os.path.exists(second)) + with open(first) as f1, open(second) as f2: + self.assertEqual(f1.read(), f2.read()) + + def assertFileContentEqual(self, file_path, expected_content): + self.assertTrue(os.path.exists(file_path)) + with open(file_path) as f: + self.assertEqual(f.read(), expected_content) + + def assertFileContentNotEqual(self, file_path, expected_content): + self.assertTrue(os.path.exists(file_path)) + with open(file_path) as f: + self.assertNotEqual(f.read(), expected_content) + def assertXMLEqual(self, act, exp): if xml_equal(act, exp): return diff --git a/tests/test_addfiles.py b/tests/test_addfiles.py index 4dba1f3f..bfee7ff4 100644 --- a/tests/test_addfiles.py +++ b/tests/test_addfiles.py @@ -66,12 +66,12 @@ class TestAddFiles(OscTestCase): """replace a deleted file ('foo')""" self._change_to_pkg('simple') p = osc.core.Package('.') - open('foo', 'w').write('replaced file\n') + with open('foo', 'w') as f: + f.write('replaced file\n') p.addfile('foo') exp = 'A foo\n' self.assertEqual(sys.stdout.getvalue(), exp) - self.assertTrue(os.path.exists(os.path.join('.osc', 'foo'))) - self.assertNotEqual(open(os.path.join('.osc', 'foo')).read(), 'replaced file\n') + self.assertFileContentNotEqual(os.path.join('.osc', 'foo'), 'replaced file\n') self.assertFalse(os.path.exists(os.path.join('.osc', '_to_be_deleted'))) self._check_status(p, 'foo', 'R') self._check_addlist('foo\n') diff --git a/tests/test_commit.py b/tests/test_commit.py index 2aab8b1e..667ab6ea 100644 --- a/tests/test_commit.py +++ b/tests/test_commit.py @@ -40,7 +40,7 @@ class TestCommit(OscTestCase): self.assertEqual(sys.stdout.getvalue(), exp) self._check_digests('testSimple_cfilesremote') self.assertTrue(os.path.exists('nochange')) - self.assertEqual(open('nochange').read(), open(os.path.join('.osc', 'nochange')).read()) + self.assertFilesEqual('nochange', os.path.join('.osc', 'nochange')) self._check_status(p, 'nochange', ' ') self._check_status(p, 'foo', ' ') self._check_status(p, 'merge', ' ') @@ -64,7 +64,7 @@ class TestCommit(OscTestCase): self.assertEqual(sys.stdout.getvalue(), exp) self._check_digests('testAddfile_cfilesremote') self.assertTrue(os.path.exists('add')) - self.assertEqual(open('add').read(), open(os.path.join('.osc', 'add')).read()) + self.assertFilesEqual('add', os.path.join('.osc', 'add')) self.assertFalse(os.path.exists(os.path.join('.osc', '_to_be_added'))) self._check_status(p, 'add', ' ') self._check_status(p, 'foo', ' ') @@ -241,7 +241,7 @@ class TestCommit(OscTestCase): self.assertEqual(sys.stdout.getvalue(), exp) self._check_digests('testAddfile_cfilesremote') self.assertTrue(os.path.exists('add')) - self.assertEqual(open('add').read(), open(os.path.join('.osc', 'add')).read()) + self.assertFilesEqual('add', os.path.join('.osc', 'add')) self.assertFalse(os.path.exists(os.path.join('.osc', '_to_be_added'))) self._check_status(p, 'add', ' ') self._check_status(p, 'foo', ' ') @@ -341,7 +341,7 @@ class TestCommit(OscTestCase): self.assertEqual(sys.stdout.getvalue(), exp) self._check_digests('testSimple_cfilesremote') self.assertTrue(os.path.exists('nochange')) - self.assertEqual(open('nochange').read(), open(os.path.join('.osc', 'nochange')).read()) + self.assertFilesEqual('nochange', os.path.join('.osc', 'nochange')) self._check_status(p, 'nochange', ' ') self._check_status(p, 'foo', ' ') self._check_status(p, 'merge', ' ') diff --git a/tests/test_init_package.py b/tests/test_init_package.py index fa0bf5f1..30519ac4 100644 --- a/tests/test_init_package.py +++ b/tests/test_init_package.py @@ -84,7 +84,8 @@ class TestInitPackage(OscTestCase): """initialize a package dir (dir is a file)""" pac_dir = os.path.join(self.tmpdir, 'testpkg') os.mkdir(pac_dir) - open(os.path.join(pac_dir, osc.core.store), 'w').write('foo\n') + with open(os.path.join(pac_dir, osc.core.store), 'w') as f: + f.write('foo\n') self.assertRaises(osc.oscerr.OscIOError, osc.core.Package.init_package, 'http://localhost', 'osctest', 'testpkg', pac_dir) if __name__ == '__main__': diff --git a/tests/test_repairwc.py b/tests/test_repairwc.py index ee58ff33..cb224b02 100644 --- a/tests/test_repairwc.py +++ b/tests/test_repairwc.py @@ -192,7 +192,7 @@ class TestRepairWC(OscTestCase): p = osc.core.Package('.', wc_check=False) p.wc_repair('http://localhost') self.assertTrue(os.path.exists(os.path.join('.osc', '_apiurl'))) - self.assertEqual(open(os.path.join('.osc', '_apiurl')).read(), 'http://localhost\n') + self.assertFileContentEqual(os.path.join('.osc', '_apiurl'), 'http://localhost\n') self.assertEqual(p.apiurl, 'http://localhost') def test_invalidapiurl(self): @@ -201,7 +201,7 @@ class TestRepairWC(OscTestCase): p = osc.core.Package('.', wc_check=False) p.wc_repair('http://localhost') self.assertTrue(os.path.exists(os.path.join('.osc', '_apiurl'))) - self.assertEqual(open(os.path.join('.osc', '_apiurl')).read(), 'http://localhost\n') + self.assertFileContentEqual(os.path.join('.osc', '_apiurl'), 'http://localhost\n') self.assertEqual(p.apiurl, 'http://localhost') def test_noapiurlNotExistingApiurl(self): @@ -223,7 +223,7 @@ class TestRepairWC(OscTestCase): prj.wc_repair('http://localhost') self.assertTrue(os.path.exists(os.path.join(storedir, '_apiurl'))) self.assertTrue(os.path.exists(os.path.join(storedir, '_apiurl'))) - self.assertEqual(open(os.path.join(storedir, '_apiurl')).read(), 'http://localhost\n') + self.assertFileContentEqual(os.path.join(storedir, '_apiurl'), 'http://localhost\n') if __name__ == '__main__': diff --git a/tests/test_request.py b/tests/test_request.py index 89807786..9bd84b7a 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -322,7 +322,7 @@ class TestRequest(OscTestCase): def test_read_request1(self): """read in a request""" - xml = open(os.path.join(self._get_fixtures_dir(), 'test_read_request1.xml')).read().strip() + xml = self._get_fixture('test_read_request1.xml') r = osc.core.Request() r.read(ET.fromstring(xml)) self.assertEqual(r.reqid, '42') @@ -353,7 +353,7 @@ class TestRequest(OscTestCase): def test_read_request2(self): """read in a request (with reviews)""" - xml = open(os.path.join(self._get_fixtures_dir(), 'test_read_request2.xml')).read().strip() + xml = self._get_fixture('test_read_request2.xml') r = osc.core.Request() r.read(ET.fromstring(xml)) self.assertEqual(r.reqid, '123') @@ -427,7 +427,7 @@ class TestRequest(OscTestCase): def test_request_list_view1(self): """test the list_view method""" - xml = open(os.path.join(self._get_fixtures_dir(), 'test_request_list_view1.xml')).read().strip() + xml = self._get_fixture('test_request_list_view1.xml') exp = """\ 62 State:new By:Admin When:2010-12-29T14:57:25 set_bugowner: buguser foo @@ -444,7 +444,7 @@ class TestRequest(OscTestCase): def test_request_list_view2(self): """test the list_view method (with history elements and description)""" - xml = open(os.path.join(self._get_fixtures_dir(), 'test_request_list_view2.xml')).read().strip() + xml = self._get_fixture('test_request_list_view2.xml') r = osc.core.Request() r.read(ET.fromstring(xml)) exp = """\ @@ -458,7 +458,7 @@ class TestRequest(OscTestCase): def test_request_str1(self): """test the __str__ method""" - xml = open(os.path.join(self._get_fixtures_dir(), 'test_request_str1.xml')).read().strip() + xml = self._get_fixture('test_request_str1.xml') r = osc.core.Request() r = osc.core.Request() r.read(ET.fromstring(xml)) @@ -555,7 +555,7 @@ Comment: """ def test_get_actions(self): """test get_actions method""" - xml = open(os.path.join(self._get_fixtures_dir(), 'test_request_list_view1.xml')).read().strip() + xml = self._get_fixture('test_request_list_view1.xml') r = osc.core.Request() r.read(ET.fromstring(xml)) sr_actions = r.get_actions('submit') diff --git a/tests/test_revertfiles.py b/tests/test_revertfiles.py index a6b93d88..3248a312 100644 --- a/tests/test_revertfiles.py +++ b/tests/test_revertfiles.py @@ -93,7 +93,7 @@ class TestRevertFiles(OscTestCase): storefile = os.path.join('.osc', fname) self.assertTrue(os.path.exists(fname)) self.assertTrue(os.path.exists(storefile)) - self.assertEqual(open(fname).read(), open(storefile).read()) + self.assertFilesEqual(fname, storefile) if __name__ == '__main__': import unittest diff --git a/tests/test_update.py b/tests/test_update.py index c0b2f168..f6c81a4b 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -115,7 +115,7 @@ class TestUpdate(OscTestCase): self.assertEqual(sys.stdout.getvalue(), exp) self._check_deletelist('foo\n') self._check_conflictlist('merge\n') - self.assertEqual(open('foo').read(), open(os.path.join('.osc', 'foo')).read()) + self.assertFilesEqual('foo', os.path.join('.osc', 'foo')) self._check_digests('testUpdateLocalDeletions_files') @GET('http://localhost/source/osctest/restore?rev=latest', file='testUpdateRestore_files') @@ -201,11 +201,9 @@ class TestUpdate(OscTestCase): exp = 'A bigfile\nD _service:exists\nA _service:bar\nA _service:foo\nAt revision 2.\n' self.assertEqual(sys.stdout.getvalue(), exp) self.assertFalse(os.path.exists(os.path.join('.osc', '_service:bar'))) - self.assertTrue(os.path.exists('_service:bar')) - self.assertEqual(open('_service:bar').read(), 'another service\n') + self.assertFileContentEqual('_service:bar', 'another service\n') self.assertFalse(os.path.exists(os.path.join('.osc', '_service:foo'))) - self.assertTrue(os.path.exists('_service:foo')) - self.assertEqual(open('_service:foo').read(), 'small\n') + self.assertFileContentEqual('_service:foo', 'small\n') self.assertTrue(os.path.exists('_service:exists')) self._check_digests('testUpdateServiceFilesAddDelete_files', '_service:foo', '_service:bar')