Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python 3 fixes - fix dirutil, fileutil, and xml_parser tests #6229

Merged
merged 4 commits into from
Jul 26, 2018
Merged

Python 3 fixes - fix dirutil, fileutil, and xml_parser tests #6229

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7ae2411
Fix dirutil tests to pass in Py3
Jul 24, 2018
856793d
Fix fileutil test
Jul 24, 2018
b546b58
Merge remote-tracking branch 'upstream/master' into py3-fixes_dirutil…
Jul 25, 2018
76bad4a
Fix XML test base binary mode
Jul 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions tests/python/pants_test/util/test_dirutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
import os
import time
import unittest
from builtins import str
from contextlib import contextmanager

import mock
from future.utils import text_type

from pants.util import dirutil
from pants.util.contextutil import pushd, temporary_dir
Expand Down Expand Up @@ -120,10 +120,10 @@ def test_safe_walk(self):
# unicode constructor.
with temporary_dir() as tmpdir:
safe_mkdir(os.path.join(tmpdir, '中文'))
if isinstance(tmpdir, text_type):
if isinstance(tmpdir, str):
tmpdir = tmpdir.encode('utf-8')
for _, dirs, _ in dirutil.safe_walk(tmpdir):
self.assertTrue(all(isinstance(dirname, text_type) for dirname in dirs))
self.assertTrue(all(isinstance(dirname, str) for dirname in dirs))

@contextmanager
def tree(self):
Expand All @@ -149,7 +149,7 @@ class Dir(datatype(['path'])):
class File(datatype(['path', 'contents'])):
@classmethod
def empty(cls, path):
return cls(path, contents=b'')
return cls(path, contents='')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throughout the tests for comparing trees, contents was being encoded as byte strings in the expected output. I'm not sure why - the tests pass when unicode in Py2, and the bytes present an issue when using Py3.


@classmethod
def read(cls, root, relpath):
Expand Down Expand Up @@ -194,11 +194,11 @@ def test_mergetree_existing(self):
self.Dir('a/b'),

# Existing overlapping file should be overlayed.
self.File('a/b/1', contents=b'1'),
self.File('a/b/1', contents='1'),

self.File.empty('a/b/2'),
self.Dir('b'),
self.File('b/1', contents=b'1'),
self.File('b/1', contents='1'),
self.File.empty('b/2'),
self.Dir('c'),

Expand Down Expand Up @@ -227,10 +227,10 @@ def test_mergetree_new(self):
self.Dir('a'),
self.File.empty('a/2'),
self.Dir('a/b'),
self.File('a/b/1', contents=b'1'),
self.File('a/b/1', contents='1'),
self.File.empty('a/b/2'),
self.Dir('b'),
self.File('b/1', contents=b'1'),
self.File('b/1', contents='1'),
self.File.empty('b/2'))

def test_mergetree_ignore_files(self):
Expand All @@ -246,7 +246,7 @@ def ignore(root, names):
self.File.empty('a/2'),
self.Dir('a/b'),
self.Dir('b'),
self.File('b/1', contents=b'1'),
self.File('b/1', contents='1'),
self.File.empty('b/2'))

def test_mergetree_ignore_dirs(self):
Expand All @@ -261,7 +261,7 @@ def ignore(root, names):
self.Dir('a'),
self.File.empty('a/2'),
self.Dir('b'),
self.File('b/1', contents=b'1'),
self.File('b/1', contents='1'),
self.File.empty('b/2'))

def test_mergetree_symlink(self):
Expand All @@ -272,7 +272,7 @@ def test_mergetree_symlink(self):
self.Dir('a'),
self.Symlink('a/2'),
self.Dir('a/b'),
self.File('a/b/1', contents=b'1'),
self.File('a/b/1', contents='1'),
self.File.empty('a/b/2'),

# NB: assert_tree does not follow symlinks and so does not descend into the
Expand Down Expand Up @@ -396,7 +396,7 @@ def test_rm_rf_no_such_file_not_an_error(self, file_name='./vanishing_file'):
def test_readwrite_file(self):
with temporary_dir() as td:
test_filename = os.path.join(td, 'test.out')
test_content = '3333'
test_content = b'3333'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be bytes for safe_file_dump() and read_file() to work.

safe_file_dump(test_filename, test_content)
self.assertEqual(read_file(test_filename), test_content)

Expand Down