WIP: Why?

bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testIdToolUpgrade.py

@@ -60,7 +60,9 @@ class TestIdToolUpgrade(ERP5TypeTestCase):
     self.tic()
 
   def beforeTearDown(self):
+    self.portal.portal_caches.clearAllCache()
     self.id_tool.clearGenerator(all=True)
+    self.tic()
 
   def createGenerators(self):
     """

bt5/erp5_core_test/TestTemplateItem/portal_components/test.erp5.testXForwardedFor.py

@@ -56,6 +56,7 @@ class TestXForwardedFor(ERP5TypeTestCase):
     response = requests.get(
       '%s/%s' % (self.portal.absolute_url(), script_id),
       headers={'X-Forwarded-For': '1.2.3.4'},
+      timeout=5,
     )
     self.assertNotEqual(response.text, '1.2.3.4')
     last_line = get_Z2_log_last_line()
@@ -63,6 +64,7 @@ class TestXForwardedFor(ERP5TypeTestCase):
     response = requests.get(
       '%s/%s' % (self.portal.absolute_url(), script_id),
       headers={'X-Forwarded-For': '1.2.3.4, 5.6.7.8'},
+      timeout=5,
     )
     self.assertNotEqual(response.text, '1.2.3.4')
     self.assertNotEqual(response.text, '5.6.7.8')
@@ -71,6 +73,7 @@ class TestXForwardedFor(ERP5TypeTestCase):
     self.assertFalse(last_line.startswith('5.6.7.8 - '), last_line)
     response = requests.get(
       '%s/%s' % (self.portal.absolute_url(), script_id),
+      timeout=5,
     )
     self.assertNotEqual(response.text, '1.2.3.4')
     last_line = get_Z2_log_last_line()
@@ -81,6 +84,7 @@ class TestXForwardedFor(ERP5TypeTestCase):
     response = requests.get(
       '%s/%s' % (self.portal.absolute_url(), script_id),
       headers={'X-Forwarded-For': '1.2.3.4'},
+      timeout=5,
     )
     self.assertEqual(response.text, '1.2.3.4')
     last_line = get_Z2_log_last_line()
@@ -88,12 +92,14 @@ class TestXForwardedFor(ERP5TypeTestCase):
     response = requests.get(
       '%s/%s' % (self.portal.absolute_url(), script_id),
       headers={'X-Forwarded-For': '1.2.3.4, 5.6.7.8'},
+      timeout=5,
     )
     self.assertEqual(response.text, '1.2.3.4')
     last_line = get_Z2_log_last_line()
     self.assertTrue(last_line.startswith('1.2.3.4 - '), last_line)
     response = requests.get(
       '%s/%s' % (self.portal.absolute_url(), script_id),
+      timeout=5,
     )
     self.assertNotEqual(response.text, '1.2.3.4')
     last_line = get_Z2_log_last_line()

bt5/erp5_crm/SkinTemplateItem/portal_skins/erp5_crm/Event_getHeadOfTextContent.py

@@ -3,6 +3,8 @@ text = context.asText()
 LENGTH = 25
 
 # TODO: Think about the display length of multibyte characters.
+if six.PY3:
+  return text[:LENGTH]
 try:
   return six.text_type(text, 'utf-8')[:LENGTH].encode('utf-8')
 except UnicodeDecodeError:

bt5/erp5_dms/TestTemplateItem/portal_components/test.erp5.testDms.py

@@ -231,6 +231,7 @@ class TestDocument(TestDocumentMixin):
       image_size = image.size
     except ImportError:
       identify_output = Popen(['identify', filename],
+                              universal_newlines=True,
                               stdout=PIPE).communicate()[0]
       image_size = tuple([int(x) for x in identify_output.split()[2].split('x')])
     os.remove(filename)

bt5/erp5_dms/TestTemplateItem/portal_components/test.erp5.testERP5WebWithDms.py

@@ -1656,6 +1656,10 @@ return True
       document_object.getId())
     response_a = self.publish(path)
     action.setVisible(0)
+    def cleanup():
+      action.setVisible(1)
+      self.tic()
+    self.addCleanup(cleanup)
     self.tic()
     response_b = self.publish(path)
     self.assertNotEqual(response_a.getBody(), response_b.getBody())
@@ -1726,6 +1730,7 @@ return True
     response = requests.get(
       self.portal.absolute_url(),
       cookies=auth_cookie,
+      timeout=10,
     )
     self.assertEqual(response.status_code, 200)
     self.assertEqual(response.headers['Cache-Control'], 'private')
@@ -1734,6 +1739,7 @@ return True
     response = requests.get(
       '%s/%s' % (website.absolute_url(), 'released_page'),
       cookies=auth_cookie,
+      timeout=10,
     )
     self.assertEqual(response.status_code, 200)
     self.assertEqual(response.headers['Cache-Control'], 'max-age=0, no-store')
@@ -1742,6 +1748,7 @@ return True
     response = requests.get(
       '%s/%s?format=txt' % (website.absolute_url(), 'released_page'),
       cookies=auth_cookie,
+      timeout=10,
     )
     self.assertEqual(response.status_code, 200)
     self.assertEqual(response.headers['Cache-Control'], 'max-age=0, no-store')
@@ -1749,12 +1756,14 @@ return True
     # published page
     response = requests.get(
       '%s/%s' % (website.absolute_url(), 'published_page'),
+      timeout=10,
     )
     self.assertEqual(response.status_code, 200)
     self.assertEqual(response.headers['Cache-Control'], 'max-age=600, stale-while-revalidate=360000, public')
     response = requests.get(
       '%s/%s' % (website.absolute_url(), 'published_page'),
       cookies=auth_cookie,
+      timeout=10,
     )
     self.assertEqual(response.status_code, 200)
     self.assertEqual(response.headers['Cache-Control'], 'max-age=0, no-store')
@@ -1762,12 +1771,14 @@ return True
     # converted published page
     response = requests.get(
       '%s/%s?format=txt' % (website.absolute_url(), 'published_page'),
+      timeout=10,
     )
     self.assertEqual(response.status_code, 200)
     self.assertEqual(response.headers['Cache-Control'], 'max-age=600, stale-while-revalidate=360000, public')
     response = requests.get(
       '%s/%s?format=txt' % (website.absolute_url(), 'published_page'),
       cookies=auth_cookie,
+      timeout=10,
     )
     self.assertEqual(response.status_code, 200)
     self.assertEqual(response.headers['Cache-Control'], 'max-age=600, stale-while-revalidate=360000, public')

bt5/erp5_dms/TestTemplateItem/portal_components/test.erp5.testWebDavSupport.py

@@ -84,11 +84,12 @@ class TestWebDavSupport(ERP5TypeTestCase):
     person = self.portal.person_module.newContent()
     self.tic()
     file_object = self.makeFileUpload('images/erp5_logo.png')
-    response = self.publish(person.getPath() + '/erp5_logo.png',
-                            request_method='PUT',
-                            stdin=file_object,
-                            basic=self.authentication)
-    self.assertEqual(response.getStatus(), httplib.CREATED)
+    response = self.publish(
+      person.getPath() + '/erp5_logo.png',
+      request_method='PUT',
+      stdin=file_object,
+      env={"CONTENT_TYPE": 'image/png'},
+      basic=self.authentication)
     self.assertEqual(response.getStatus(), six.moves.http_client.CREATED)
     image = person['erp5_logo.png']
     self.assertEqual(image.getPortalType(), 'Embedded File')
@@ -104,10 +105,12 @@ class TestWebDavSupport(ERP5TypeTestCase):
     path = self.portal.portal_contributions.getPath()
     filename = 'P-DMS-Presentation.3.Pages-001-en.odp'
     file_object = self.makeFileUpload(filename)
-    response = self.publish('%s/%s' % (path, filename),
-                            request_method='PUT',
-                            stdin=file_object,
-                            basic=self.authentication)
+    response = self.publish(
+      '%s/%s' % (path, filename),
+      request_method='PUT',
+      stdin=file_object,
+      env={"CONTENT_TYPE": 'application/vnd.oasis.opendocument.presentation'},
+      basic=self.authentication)
 
     self.assertEqual(response.getStatus(), six.moves.http_client.CREATED)
     document_module = self.getDocumentModule()
@@ -126,10 +129,12 @@ class TestWebDavSupport(ERP5TypeTestCase):
     path = self.portal.portal_contributions.getPath()
     filename = 'P-DMS-Presentation.3.Pages-001-en.odp'
     file_object = self.makeFileUpload(filename)
-    response = self.publish('%s/%s' % (path, filename),
-                            request_method='PUT',
-                            stdin=file_object,
-                            basic=self.authentication)
+    response = self.publish(
+      '%s/%s' % (path, filename),
+      request_method='PUT',
+      stdin=file_object,
+      env={"CONTENT_TYPE": 'application/vnd.oasis.opendocument.presentation'},
+      basic=self.authentication)
 
     self.assertEqual(response.getStatus(), six.moves.http_client.CREATED)
     self.tic()
@@ -198,10 +203,12 @@ class TestWebDavSupport(ERP5TypeTestCase):
     path = self.portal.portal_contributions.getPath()
     filename = 'P-DMS-Presentation.3.Pages-001-en.odp'
     file_object = self.makeFileUpload(filename)
-    response = self.publish('%s/%s' % (path, filename),
-                            request_method='PUT',
-                            stdin=file_object,
-                            basic=self.authentication)
+    response = self.publish(
+      '%s/%s' % (path, filename),
+      request_method='PUT',
+      stdin=file_object,
+      env={"CONTENT_TYPE": 'application/vnd.oasis.opendocument.presentation'},
+      basic=self.authentication)
     # Convert to base format and run conversion into utf-8
     self.tic()
 
@@ -225,10 +232,12 @@ class TestWebDavSupport(ERP5TypeTestCase):
     path = self.portal.portal_contributions.getPath()
     filename = 'P-DMS-Presentation.3.Pages-001-en.odp'
     file_object = self.makeFileUpload(filename)
-    response = self.publish('%s/%s' % (path, filename),
-                            request_method='PUT',
-                            stdin=file_object,
-                            basic=self.authentication)
+    response = self.publish(
+      '%s/%s' % (path, filename),
+      request_method='PUT',
+      stdin=file_object,
+      env={"CONTENT_TYPE": 'application/vnd.oasis.opendocument.presentation'},
+      basic=self.authentication)
     document_module = self.getDocumentModule()
     document = document_module[filename]
 
@@ -263,10 +272,12 @@ class TestWebDavSupport(ERP5TypeTestCase):
     path = self.portal.portal_contributions.getPath()
     filename = 'P-DMS-Presentation.3.Pages-001-en.odp'
     file_object = self.makeFileUpload(filename)
-    response = self.publish('%s/%s' % (path, filename),
-                            request_method='PUT',
-                            stdin=file_object,
-                            basic=self.authentication)
+    response = self.publish(
+      '%s/%s' % (path, filename),
+      request_method='PUT',
+      stdin=file_object,
+      env={"CONTENT_TYPE": 'application/vnd.oasis.opendocument.presentation'},
+      basic=self.authentication)
     document_module = self.getDocumentModule()
     document = document_module[filename]
 

bt5/erp5_forge/ModuleComponentTemplateItem/portal_components/module.erp5.Git.py

@@ -64,6 +64,7 @@ class Git(WorkingCopy):
 
   def _git(self, *args, **kw):
     kw.setdefault('cwd', self.working_copy)
+    kw.setdefault('universal_newlines', True)
     argv = ['git']
     try:
       return subprocess.Popen(argv + list(args), **kw)
@@ -362,7 +363,6 @@ class Git(WorkingCopy):
             raise
           # try to update our working copy
           # TODO: find a solution if there are other local changes
-          # TODO: solve conflicts on */bt/revision automatically
           try:
             self.git(merge, '@{u}', env=env)
           except GitError as e2:

bt5/erp5_stripe/TestTemplateItem/portal_components/test.erp5.testStripe.py

@@ -26,7 +26,6 @@
 ##############################################################################
 
 import json
-from six.moves import urllib
 from io import BytesIO
 from six.moves.urllib.parse import parse_qs
 
@@ -461,8 +460,8 @@ class TestStripePaymentSession(ERP5TypeTestCase):
       )
       ret = self.publish(
         "%s/ERP5Site_receiveStripeWebHook" % self.portal.getPath(),
-        stdin=BytesIO(urllib.parse.urlencode({
-          "BODY": json.dumps({
+        stdin=BytesIO(
+          json.dumps({
             "url": "https://stripe.url",
             "id": "evt_%s" % "abc321_expired",
             "object": "event",
@@ -474,9 +473,10 @@ class TestStripePaymentSession(ERP5TypeTestCase):
                 "object": "checkout.session"
               }
             }
-          })
-        }).encode()),
+          }).encode()
+        ),
         request_method="POST",
+        env={'CONTENT_TYPE': 'application/json'},
         handle_errors=False)
       self.assertEqual(200, ret.getStatus())
       self.tic()
@@ -658,8 +658,8 @@ class TestStripePaymentSession(ERP5TypeTestCase):
       )
       ret = self.publish(
         "%s/ERP5Site_receiveStripeWebHook" % self.portal.getPath(),
-        stdin=BytesIO(urllib.parse.urlencode({
-          "BODY": json.dumps({
+        stdin=BytesIO(
+          json.dumps({
             "id": "evt_%s" % session_id,
             "object": "event",
             "data": {
@@ -670,9 +670,10 @@ class TestStripePaymentSession(ERP5TypeTestCase):
                 "object": "checkout.session"
               }
             }
-          })
-        }).encode()),
+          }).encode(),
+        ),
         request_method="POST",
+        env={'CONTENT_TYPE': 'application/json'},
         handle_errors=False)
       self.assertEqual(200, ret.getStatus())
       self.tic()

bt5/erp5_ui_test_core/ExtensionTemplateItem/portal_components/extension.erp5.MailHostUtils.py

@@ -41,4 +41,4 @@ def setupDummyMailHost(self):
     cls.__bases__ = (DummyMailHostMixin,) + cls.__bases__
     pmc_init_of(cls)
   mailhost.reset()
-  return True
+  return "True"  # TODO: zope4py3 Zope does not seem to publish `True` correctly

bt5/erp5_web/TestTemplateItem/portal_components/test.erp5.testERP5Web.py

@@ -1334,6 +1334,7 @@ Hé Hé Hé!""", page.asText().strip())
     conditional_get_response = requests.get(
       web_section.absolute_url(),
       headers={'If-Modified-Since': DateTime().utcdatetime().strftime('%a, %d %b %Y %H:%M:%S UTC')},
+      timeout=5,
     )
     self.assertEqual(conditional_get_response.status_code, 304)
     self.assertIn('Cache-Control', conditional_get_response.headers)

product/ERP5OOo/tests/testOOoImport.py

@@ -86,6 +86,7 @@ class TestOOoImportMixin(ERP5TypeTestCase):
         self.portal.portal_categories.gender,
         self.portal.portal_categories.region,
         ]:
+      parent.setLastId('0')
       parent.deleteContent(list(parent.objectIds()))
     self.tic()
 

product/ERP5Type/tests/ERP5TypeTestCase.py

@@ -33,6 +33,8 @@ from Products.ZMySQLDA.DA import Connection as ZMySQLDA_Connection
 from zope.globalrequest import getRequest
 from zope.globalrequest import setRequest
 import six
+if six.PY3:
+  StandardError = Exception
 
 from zope.component.hooks import setSite
 

product/PortalTransforms/Transform.py

@@ -22,7 +22,14 @@ from Products.PortalTransforms.transforms.broken import BrokenTransform
 
 def import_from_name(module_name):
     """ import and return a module by its name """
-    return import_module(module_name)
+    __traceback_info__ = (module_name,)
+    m = import_module(module_name)
+    try:
+        for sub in module_name.split(".")[1:]:
+            m = getattr(m, sub)
+    except AttributeError as e:
+        raise ImportError(str(e))
+    return m
 
 def make_config_persistent(kwargs):
     """ iterates on the given dictionnary and replace list by persistent list,

product/ZSQLCatalog/tests/testSQLCatalog.py

@@ -266,12 +266,10 @@ class TestSQLCatalog(ERP5TypeTestCase):
     self.assertRaises(exception, self._catalog, src__=1, query_table='foo', **kw)
 
   def catalog(self, reference_tree, kw, check_search_text=True,
-      check_select_expression=True, expected_failure=False):
+      check_select_expression=True):
     reference_param_dict = self._catalog.buildSQLQuery(query_table='foo', **kw)
     query = self._catalog.buildEntireQuery(kw).query
     assertEqual = self.assertEqual
-    if expected_failure:
-      assertEqual = unittest.expectedFailure(assertEqual)
 
     assertEqual(reference_tree, query)
     search_text = query.asSearchTextExpression(self._catalog)

[Jérome Perrin]

Hello @arnau I'm replying here to the "why" for what I remember.

here this was to be able to run the test more than once

[Arnaud Fontaine]

@jerome Of course, you mean as a Live Test right?

[Jérome Perrin]

yes

[Arnaud Fontaine]

Thanks. I will create a separate commit to be merged into master.

[Jérome Perrin]

this is for a new pylint warning ( https://pylint.pycqa.org/en/latest/user_guide/messages/warning/missing-timeout.html )

[Arnaud Fontaine]

Thanks!

[Jérome Perrin]

this is because on py3 we have str so we don't need to worry that we cut in the middle of multibyte character

python3

>>> print("héhé"[:2])
hé

python2

>>> print("héhé"[:2])
h�

because

>>> print(repr("héhé"))
'h\xc3\xa9h\xc3\xa9'
>>> print(repr("héhé"[:2]))
'h\xc3'

[Arnaud Fontaine]

Thanks!

[Jérome Perrin]

this is to have stdout in text mode

[Arnaud Fontaine]

Thanks!

[Jérome Perrin]

to be able to run the test more than once

[Jérome Perrin]

with zope5 (I don't know exactly since what), the content type header seems to be needed for webdav ( RFC says "should" https://datatracker.ietf.org/doc/html/rfc4918#section-9.7.1 but here it's "must" ). Because we don't use webdav so much it seems no problem.

[Arnaud Fontaine]

I checked in Zope source code but I could see any place where this is a "must"...

[Jérome Perrin]

I wrote this from memory, but maybe I did not even check zope code. I feel this is "good enough" to commit this without too much details because webdav is not critical

[Arnaud Fontaine]

I investigated a bit. It seems to be that since the following Zope.git commit, a missing CONTENT_TYPE means application/x-www-form-urlencoded (whereas the previous implementation using cgi.FieldStorage was considering this as text/plain):

commit 5b324f6c461f5ea1cc069739b6c32a1a5ff59df9
Author: Dieter Maurer <d-maurer@users.noreply.github.com>
Date:   Thu Jan 19 07:15:18 2023 +0100

    replace `cgi.FieldStorage` by `multipart` (#1094)
    
    * replace `cgi.FieldStorage` by `multipart`
    * interpret a missing `CONTENT_TYPE` as `application/x-www-form-urlencoded` (required by some tests

And the backtrace FTR:

Traceback (most recent call last):
  File "erp5://portal_components/test.erp5.testWebDavSupport", line 232, in test_PROPFIND_on_document
    response = self.publish(
  File "/opt/slapgrid/3f9add9291086dee302fc478df4b3130/parts/erp5/product/ERP5Type/tests/ERP5TypeTestCase.py", line 793, in publish
    return super(ERP5TypeTestCaseMixin, self).publish(
  File "/opt/slapgrid/3f9add9291086dee302fc478df4b3130/eggs/Zope-5.10-py3.9.egg/Testing/ZopeTestCase/functional.py", line 42, in wrapped_func
    return func(*args, **kw)
  File "/opt/slapgrid/3f9add9291086dee302fc478df4b3130/eggs/Zope-5.10-py3.9.egg/Testing/ZopeTestCase/functional.py", line 133, in publish
    wsgi_result = publish(env, start_response)
  File "/opt/slapgrid/3f9add9291086dee302fc478df4b3130/eggs/Zope-5.10-py3.9.egg/ZPublisher/httpexceptions.py", line 30, in __call__
    return self.application(environ, start_response)
  File "/opt/slapgrid/3f9add9291086dee302fc478df4b3130/eggs/Zope-5.10-py3.9.egg/ZPublisher/WSGIPublisher.py", line 391, in publish_module
    response = _publish(request, new_mod_info)
  File "/opt/slapgrid/3f9add9291086dee302fc478df4b3130/parts/erp5/product/ERP5Type/patches/WSGIPublisher.py", line 150, in publish
    return _original_publish(request, module_info)
  File "/opt/slapgrid/3f9add9291086dee302fc478df4b3130/eggs/Zope-5.10-py3.9.egg/ZPublisher/WSGIPublisher.py", line 251, in publish
    request.processInputs()
  File "/opt/slapgrid/3f9add9291086dee302fc478df4b3130/parts/erp5/product/Localizer/patches.py", line 37, in new_processInputs
    HTTPRequest.old_processInputs(self)
  File "/opt/slapgrid/3f9add9291086dee302fc478df4b3130/eggs/Zope-5.10-py3.9.egg/ZPublisher/HTTPRequest.py", line 544, in processInputs
    key = item.name.encode("latin-1").decode(
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe3 in position 10: invalid continuation byte

[Jérome Perrin]

Thanks a lot. I agree that if tests pass without this, then no reason to have the change

[Arnaud Fontaine]

Sorry, I didn't check carefully enough and it raises an error (see my edited comment above).

[Arnaud Fontaine]

So it was working by chance before... I will apply this commit separately to keep a track of my investigation.

[Jérome Perrin]

ah yes, I remember vaguely now. It's good to have the investigation in commit message yes

[Jérome Perrin]

a TODO fixed more than 10 years ago

[Arnaud Fontaine]

Thanks!

[Jérome Perrin]

If I remember correctly, this fake request was wrong, but on python2 it was equivalent

[Jérome Perrin]

This is similar to "publish() stdin is now BytesIO rather than StringIO." from d8a59d7d , I believe we can put this part in the same commit

[Arnaud Fontaine]

Thanks!

[Jérome Perrin]

I wrote this TODO long time ago, I think we don't care about this TODO comment. On python2, whatever is returned by a published method of script is passed to str(), but not on python3, responses must be strings or bytes and while strictly speaking a bit incompatible, this seems better this way

[Jérome Perrin]

( also to run the test more than once )

[Jérome Perrin]

maybe to fix a pyint warning

[Arnaud Fontaine]

Thanks. I removed this bit because StandardError is not referenced at all in that file and not imported from somewhere else neither.

[Jérome Perrin]

ah yes it seems this is no longer needed now that we pushed 87f6c14c to master

[Jérome Perrin]

why ? ... maybe to have a better traceback during debugging.

[Arnaud Fontaine]

Strange... I did remove that code in dab26b0d (first by using __import__() and then import_module() in a later commit). Anyhow, this is not related to python3.

[Jérome Perrin]

expected failure work differently on python3, only the test method can be marked expected failure

[Jérome Perrin]

I remember we had to change other tests because of this

[Arnaud Fontaine]

Thanks! I put all these changes in the py2/py3: Make Products code compatible with both python2 and python3. and added a line in the commit message about this.

[Jérome Perrin]

that's good