Hi Bern:

I needed to change two test cases in test/test_cgi.py as well:

   FormTestCase.testCSVExportBadColumnName
   FormTestCase.testCSVExportFailPermission

they both expected exceptions.SeriousError and I am now raising
exceptions.NotFound.

test_cgi.py::testCSVExportFailPermission was changed on issue2550712
(exportcsvaction errors poorly when given invalid columns). Ced worked
on the patch for this as well as you.

The diff for the test on commit 21705126dafa for issue2550712 is:

     def testCSVExportFailPermission(self):
         cl = self._make_client({'@columns': 'id,email,password'},
nodeid=None,
             userid='2')
@@ -937,7 +947,9 @@
         output = StringIO.StringIO()
         cl.request = MockNull()
         cl.request.wfile = output
-        self.assertRaises(exceptions.Unauthorised,
+        # used to be self.assertRaises(exceptions.Unauthorised,
+        # but not acting like the column name is not found
+        self.assertRaises(exceptions.SeriousError,
             actions.ExportCSVAction(cl).handle)

The name of the test makes me think that Unauthorised should be
raised always. If the user user2 can't access the user class, how
could they find out that the column doesn't exist?

I added ced to this ticket since he worked on the patch.
Maybe the patch for that issue needs to be tweaked?

Thoughts?

I have checked in the change from exceptions.SeriousError to
exceptions.NotFound to get the 2 tests to pass again.

I also renamed

  test/test_cgi.py::FormTestCase::testCSVExportFailPermission

to

  test/test_cgi.py::FormTestCase::testCSVExportFailPermissionBadColumn

and added

 test/test_cgi.py::FormTestCase::testCSVExportFailPermissionValidColumn

that verifies that exceptions.Unauthorised is raised.

I am leaving this ticket open pending a decision on what should be
raised with the testCSVExportFailPermissionBadColumn test.