Skip to content

Commit

Permalink
Ensure client generation works smoothly in py3.
Browse files Browse the repository at this point in the history
It turns out that generated clients had a few issues in python3:

* the discovery doc was returned as bytes, but `json.loads` wanted a string
* in python3, source files should be strings, not bytes
* \N, \u, and \U have special interpretations in python3 strings, which makes
  them invalid bare sequences in a docstring.

This commit updates these and re-enables tests for python3. The only remaining
py2-only check is the "did we generate the exact same source file" check,
which fails for trivial reasons (eg u'foo' vs. 'foo') for python3.

Since there are no more python27-only tests, I removed the "disable test in
2.7" helper.
  • Loading branch information
craigcitro committed Mar 29, 2018
1 parent ed6f279 commit d475ca9
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 12 deletions.
10 changes: 6 additions & 4 deletions apitools/gen/client_generation_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@
import importlib
import logging
import os
import six
import subprocess
import sys
import tempfile

import unittest2

from apitools.gen import gen_client
from apitools.gen import test_utils

if six.PY2:
import unittest2 as unittest
else:
import unittest

_API_LIST = [
'bigquery.v2',
Expand All @@ -36,14 +39,13 @@
]


class ClientGenerationTest(unittest2.TestCase):
class ClientGenerationTest(unittest.TestCase):

def setUp(self):
super(ClientGenerationTest, self).setUp()
self.gen_client_binary = 'gen_client'

@test_utils.SkipOnWindows
@test_utils.RunOnlyOnPython27
def testGeneration(self):
for api in _API_LIST:
with test_utils.TempDir(change_to=True):
Expand Down
1 change: 0 additions & 1 deletion apitools/gen/gen_client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ def _GetContent(file_path):
return f.read()


@test_utils.RunOnlyOnPython27
class ClientGenCliTest(unittest2.TestCase):

def testHelp_NotEnoughArguments(self):
Expand Down
5 changes: 1 addition & 4 deletions apitools/gen/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,14 @@

import contextlib
import os
import tempfile
import shutil
import sys
import tempfile

import six
import unittest2


RunOnlyOnPython27 = unittest2.skipUnless(
sys.version_info[:2] == (2, 7), 'Only runs in Python 2.7')

SkipOnWindows = unittest2.skipIf(
os.name == 'nt', 'Does not run on windows')

Expand Down
13 changes: 11 additions & 2 deletions apitools/gen/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,11 @@ def CleanDescription(description):
"""Return a version of description safe for printing in a docstring."""
if not isinstance(description, six.string_types):
return description
if six.PY3:
# https://docs.python.org/3/reference/lexical_analysis.html#index-18
description = description.replace('\\N', '\\\\N')
description = description.replace('\\u', '\\\\u')
description = description.replace('\\U', '\\\\U')
return description.replace('"""', '" " "')


Expand Down Expand Up @@ -298,7 +303,8 @@ def __call__(self, *args):
line = (args[0] % args[1:]).rstrip()
else:
line = args[0].rstrip()
line = line.encode('ascii', 'backslashreplace')
if six.PY2:
line = line.encode('ascii', 'backslashreplace')
print('%s%s' % (self.__indent, line), file=self.__out)
else:
print('', file=self.__out)
Expand Down Expand Up @@ -327,7 +333,10 @@ def FetchDiscoveryDoc(discovery_url, retries=5):
for url in discovery_urls:
for _ in range(retries):
try:
discovery_doc = json.loads(urllib_request.urlopen(url).read())
content = urllib_request.urlopen(url).read()
if isinstance(content, bytes):
content = content.decode('utf8')
discovery_doc = json.loads(content)
break
except (urllib_error.HTTPError, urllib_error.URLError) as e:
logging.info(
Expand Down
6 changes: 5 additions & 1 deletion samples/uptodate_check_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import os
import difflib

import six
import unittest2

from apitools.gen import gen_client
Expand All @@ -30,7 +31,6 @@ def _GetContent(file_path):
return f.read()


@test_utils.RunOnlyOnPython27
class ClientGenCliTest(unittest2.TestCase):

def AssertDiffEqual(self, expected, actual):
Expand Down Expand Up @@ -59,6 +59,10 @@ def _CheckGeneratedFiles(self, api_name, api_version):
prefix + '_messages.py',
'__init__.py']))
self.assertEquals(expected_files, set(os.listdir(tmp_dir_path)))
if six.PY3:
# The source files won't be identical under python3,
# so we exit early.
return
for expected_file in expected_files:
self.AssertDiffEqual(
_GetContent(GetSampleClientPath(
Expand Down

0 comments on commit d475ca9

Please sign in to comment.