Merge lp:~james-w/pkgme/better-json-error-message into lp:pkgme

[James Westby]

Hi,

Another simple fix of the "improve error message" variety.

I don't like the repr much, but not sure what's the best thing to do.

Thanks,

James

[Jonathan Lange (jml)]

Definitely an improvement. Don't know what to do about the repr().

review: Approve

=== modified file 'pkgme/project_info.py'
--- pkgme/project_info.py	2011-08-10 14:57:02 +0000
+++ pkgme/project_info.py	2011-10-31 16:41:28 +0000
@@ -1,4 +1,5 @@
 import json
+import os
 from pprint import pformat
 
 from pkgme.run_script import run_script, ScriptMissing
@@ -61,7 +62,12 @@
         out = run_script(
             self.basepath, self.INFO_SCRIPT_NAME, self.cwd,
             to_write=json.dumps(keys))
-        ret = json.loads(out)
+        try:
+            ret = json.loads(out)
+        except ValueError:
+            raise AssertionError("%s didn't return valid JSON: %r"
+                    % (os.path.join(self.basepath, self.INFO_SCRIPT_NAME),
+                        out))
         trace.debug('%s returned %s' % (self.INFO_SCRIPT_NAME, pformat(ret)))
         return ret
 

=== modified file 'pkgme/tests/test_project_info.py'
--- pkgme/tests/test_project_info.py	2011-08-10 14:53:01 +0000
+++ pkgme/tests/test_project_info.py	2011-10-31 16:41:28 +0000
@@ -131,6 +131,22 @@
         info = SingleExternalHelperInfo(tempdir.path, tempdir.path)
         self.assertEqual({script_name: value}, info.get_all([script_name]))
 
+    def test_when_script_gives_invalid_json_output(self):
+        """Test when the script doesn't produce valid JSON.
+
+        As this is a problem with the backend we expect an AssertionError
+        with an intelligible message.
+        """
+        tempdir = self.useFixture(TempdirFixture())
+        script_path = tempdir.abspath(SingleExternalHelperInfo.INFO_SCRIPT_NAME)
+        value = self.getUniqueString()
+        script = '#!%s\nprint "}Nonsense"' % (sys.executable, )
+        self.useFixture(ExecutableFileFixture(script_path, script))
+        info = SingleExternalHelperInfo(tempdir.path, tempdir.path)
+        e = self.assertRaises(AssertionError, info.get_all, [])
+        self.assertEquals("%s didn't return valid JSON: '}Nonsense\\n'"
+                % (script_path, ), str(e))
+
     def test_passes_input(self):
         tempdir = self.useFixture(TempdirFixture())
         script_name = self.getUniqueString()