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.

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

Drop dependency on 'google.apputils'. #165

Closed

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Jan 13, 2015

Use stdlib's 'unittest' instead.

Use stdlib's 'unittest' instead.
@liujisi
Copy link
Contributor

liujisi commented Feb 23, 2015

@haberman

@tamird
Copy link
Contributor

tamird commented Apr 6, 2015

seems like a no-brainer. @tseaver can you rebase?

@haberman
Copy link
Member

haberman commented Apr 6, 2015

Unfortunately I don't think the rebase will be trivial. I wrote more about this here: #166 (comment)

@haberman
Copy link
Member

haberman commented Apr 6, 2015

Specifically we've started depending on a feature of google-apputils test framework. parameterized.Parameters lets us easily parameterize many tests to verify they work with both proto2 and proto3.

I'd be happy to see us drop the google-apputils dependency, but we'd need a way to accommodate the need that parameterized.Parameters is currently serving.

@tamird
Copy link
Contributor

tamird commented Apr 6, 2015

Looks like this repo doesn't actually use upstream's parameterized? https://github.com/google/protobuf/blob/master/python/google/protobuf/internal/_parameterized.py

@haberman
Copy link
Member

haberman commented Apr 6, 2015

It's true. We discovered that the open-source release of google-apputils hasn't actually been updated in a while, so it doesn't include parameterized. We had to include our own copy in this repo.

@tamird
Copy link
Contributor

tamird commented Apr 6, 2015

Does that mean that this rebase will be relatively straightforward, then?

@haberman
Copy link
Member

haberman commented Apr 6, 2015

No because our copy of parameterized depends on google-apputils.

https://github.com/google/protobuf/blob/master/python/google/protobuf/internal/_parameterized.py#L155

@tamird
Copy link
Contributor

tamird commented Apr 6, 2015

@haberman pardon my ignorance - how does one even run the python tests? ah, the readme is accurate if basetest is used, but not after this PR's modifications.

@haberman
Copy link
Member

haberman commented Apr 6, 2015

Instructions/docs for this are in the Python README: https://github.com/google/protobuf/tree/master/python

@tamird
Copy link
Contributor

tamird commented Apr 7, 2015

Hm, well I'm trying to rebase this and get the tests running - with the following change:

diff --git a/python/setup.py b/python/setup.py
index c18818e..cf22b75 100755
--- a/python/setup.py
+++ b/python/setup.py
@@ -165,7 +165,7 @@ if __name__ == '__main__':
         version = GetVersion(),
         packages = [ 'google' ],
         namespace_packages = [ 'google' ],
-        google_test_dir = "google/protobuf/internal",
+        test_suite = 'google.protobuf.internal',
         # Must list modules explicitly so that we don't install tests.
         py_modules = [
           'google.protobuf.internal.api_implementation',
@@ -195,11 +195,6 @@ if __name__ == '__main__':
           'google.protobuf.text_format'],
         cmdclass = { 'clean': clean, 'build_py': build_py },
         install_requires = ['setuptools'],
-        # TODO: Restore dependency once a Python 3 compatible google-apputils
-        # is released.
-        setup_requires = (['google-apputils']
-                          if sys.version_info[0] < 3 else
-                          []),
         ext_modules = ext_module_list,
         url = 'https://developers.google.com/protocol-buffers/',
         maintainer = maintainer_email,

I get the following error:

[~/src/protobuf/python] python setup.py test
Traceback (most recent call last):
  File "setup.py", line 205, in <module>
    "Protocol Buffers are Google's data interchange format.",
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/core.py", line 151, in setup
    dist.run_commands()
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/dist.py", line 953, in run_commands
    self.run_command(cmd)
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/dist.py", line 972, in run_command
    cmd_obj.run()
  File "/usr/local/lib/python2.7/site-packages/setuptools/command/test.py", line 142, in run
    self.with_project_on_sys_path(self.run_tests)
  File "/usr/local/lib/python2.7/site-packages/setuptools/command/test.py", line 122, in with_project_on_sys_path
    func()
  File "/usr/local/lib/python2.7/site-packages/setuptools/command/test.py", line 163, in run_tests
    testRunner=self._resolve_as_ep(self.test_runner),
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/main.py", line 94, in __init__
    self.parseArgs(argv)
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/main.py", line 149, in parseArgs
    self.createTests()
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/main.py", line 158, in createTests
    self.module)
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/loader.py", line 132, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/loader.py", line 105, in loadTestsFromName
    return self.loadTestsFromModule(obj)
  File "/usr/local/lib/python2.7/site-packages/setuptools/command/test.py", line 37, in loadTestsFromModule
    tests.append(self.loadTestsFromName(submodule))
  File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/loader.py", line 102, in loadTestsFromName
    parent, obj = obj, getattr(obj, part)
AttributeError: 'module' object has no attribute 'cpp_message'

Any idea what's going on?

@haberman
Copy link
Member

haberman commented Apr 7, 2015

It looks like it is failing to load one of the test modules for some reason. These are probably the kinds of errors that will need to be sorted through for this change to be workable.

@tseaver tseaver deleted the drop-apputils-dependency branch May 11, 2015 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants