Skip to content

Conversation

shin-
Copy link
Contributor

@shin- shin- commented Sep 23, 2015

Also includes a few unit tests for utils.convert_volume_binds

Fixes #782

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be able to avoid this step if we go:

expected = [six.u('/mnt/지연:/unicode/박:rw')]

@shin- shin- force-pushed the 782-unicode-volume-paths branch from c822470 to 332d1ea Compare September 24, 2015 16:57
@shin-
Copy link
Contributor Author

shin- commented Sep 24, 2015

@aanand Thanks, didn't know about six.u. PTAL?

@aanand
Copy link
Contributor

aanand commented Sep 24, 2015

Cool. Perhaps the first test should be rewritten so it tests byte string input on all Python versions:

    def test_convert_volume_bindings_binary_input(self):
        expected = [six.u('/mnt/지연:/unicode/박:rw')]

        data = {
            b'/mnt/지연': {
                'bind': b'/unicode/박',
                'mode': 'rw'
            }
        }
        self.assertEqual(
            convert_volume_binds(data), expected
        )

@shin-
Copy link
Contributor Author

shin- commented Sep 24, 2015

Okay, I've tried all sorts of combination, I've settled on just separating py2 and py3 tests completely (so it's 2+2 now). On the bright side, results were consistent all along (and we have tests to prove it)!

@aanand
Copy link
Contributor

aanand commented Sep 25, 2015

OK, I see the rationale, but instead of entirely separate test methods, could we just switch on the Python version inside the method?

def test_convert_volume_binds_unicode_bytes_input(self):
    if six.PY2:
        expected = [unicode('/mnt/지연:/unicode/박:rw', 'utf-8')]
        data = {
            '/mnt/지연': {
                'bind': '/unicode/박',
                'mode': 'rw'
            }
        }
    else:
        expected = ['/mnt/지연:/unicode/박:rw']
        data = {
            bytes('/mnt/지연', 'utf-8'): {
                'bind': bytes('/unicode/박', 'utf-8'),
                'mode': 'rw'
            }
        }

    self.assertEqual(convert_volume_binds(data), expected)

def test_convert_volume_binds_unicode_unicode_input(self):
    if six.PY2:
        expected = [unicode('/mnt/지연:/unicode/박:rw', 'utf-8')]
        data = {
            unicode('/mnt/지연', 'utf-8'): {
                'bind': unicode('/unicode/박', 'utf-8'),
                'mode': 'rw'
            }
        }
    else:
        expected = ['/mnt/지연:/unicode/박:rw']
        data = {
            '/mnt/지연': {
                'bind': '/unicode/박',
                'mode': 'rw'
            }
        }

    self.assertEqual(convert_volume_binds(data), expected)

Also includes a few unit tests for utils.convert_volume_binds

Signed-off-by: Joffrey F <joffrey@docker.com>
@shin- shin- force-pushed the 782-unicode-volume-paths branch from b2e9144 to 29219a2 Compare September 25, 2015 21:36
shin- added a commit that referenced this pull request Oct 5, 2015
Don't break when volume binds contain unicode characters
@shin- shin- merged commit 73e45ef into master Oct 5, 2015
@shin- shin- deleted the 782-unicode-volume-paths branch November 12, 2015 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants