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

Config with default template #121

Merged
merged 9 commits into from
Mar 14, 2023

Conversation

gforcada
Copy link
Sponsor Member

@ale-rt some tests fail to run because zpretty reformatted a few files and added a utf-8 encoding declaration and lxml refuses to parse them as text, but suggests that they should be send to lxml as bytes rather.

Should we adapt the tests to parse the files as bytes, or rather should zpretty not add those utf-8 encoding declarations? 🤔

Error in test test_get_vocabularies (plone.app.querystring.tests.testRegistryReader.TestRegistryReader.test_get_vocabularies)
Traceback (most recent call last):
  File "/usr/lib64/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/usr/lib64/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
  File "/usr/lib64/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
  File "plone.app.querystring/plone/app/querystring/tests/testRegistryReader.py", line 73, in test_get_vocabularies
    registry = self.createRegistry(td.test_vocabulary_xml)
  File "plone.app.querystring/plone/app/querystring/tests/testRegistryReader.py", line 47, in createRegistry
    importer.importDocument(xml)
  File "plone.app.querystring/.tox/test/lib/python3.11/site-packages/plone/app/registry/exportimport/handler.py", line 109, in importDocument
    tree = etree.fromstring(document)
  File "src/lxml/etree.pyx", line 3257, in lxml.etree.fromstring
  File "src/lxml/parser.pxi", line 1911, in lxml.etree._parseMemoryDocument
ValueError: Unicode strings with encoding declaration are not supported. Please use bytes input or XML fragments without declaration.

🍀

@mister-roboto
Copy link

@gforcada thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

@gforcada gforcada force-pushed the config-with-default-template-dcfc6395 branch from 786ee73 to 7daf749 Compare March 13, 2023 01:43
Copy link
Sponsor Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

Should we adapt the tests to parse the files as bytes, or rather should zpretty not add those utf-8 encoding declarations? 🤔

I added a commit to turn the test inputs into bytes first, then it works.

@mauritsvanrees
Copy link
Sponsor Member

@mauritsvanrees
Copy link
Sponsor Member

Jenkins passes, but the gh-actions tests fail, also locally:

Failure in test test__navigationPath (plone.app.querystring.tests.testQueryParser.TestQueryGenerators)
Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.10/3.10.10_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/local/Cellar/python@3.10/3.10.10_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py", line 591, in run
    self._callTestMethod(testMethod)
  File "/usr/local/Cellar/python@3.10/3.10.10_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py", line 549, in _callTestMethod
    method()
  File "/Users/maurits/community/plone-coredev/6.0/src/plone.app.querystring/plone/app/querystring/tests/testQueryParser.py", line 535, in test__navigationPath
    self.assertEqual(parsed, expected)
  File "/usr/local/Cellar/python@3.10/3.10.10_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py", line 845, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/local/Cellar/python@3.10/3.10.10_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py", line 1144, in assertDictEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/local/Cellar/python@3.10/3.10.10_1/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py", line 675, in fail
    raise self.failureException(msg)
AssertionError: {'path': {'query': ['//bar/']}} != {'path': {'query': ['/site/foo/bar/']}}
- {'path': {'query': ['//bar/']}}
+ {'path': {'query': ['/site/foo/bar/']}}
?                       ++++++++

@mauritsvanrees
Copy link
Sponsor Member

Ah, right, we need a new release of plone.app.layout in 6.0-dev. I will see to that.

@mauritsvanrees mauritsvanrees merged commit 2374e2f into master Mar 14, 2023
@mauritsvanrees mauritsvanrees deleted the config-with-default-template-dcfc6395 branch March 14, 2023 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants