Update test: plone.app.discussion #843

ksuess · 2019-12-10T08:20:57Z

plone.app.discussion introduces an additional workflow. This change takes the additional workflow states into account.

mister-roboto · 2019-12-10T08:21:05Z

@ksuess thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

coveralls · 2019-12-10T09:09:40Z

Coverage remained the same at ?% when pulling fcba527 on plone.app.discussion-moderation into de8614a on master.

coveralls · 2019-12-10T09:09:40Z

Coverage remained the same at ?% when pulling 2ade84b on plone.app.discussion-moderation into de8614a on master.

coveralls · 2019-12-10T09:09:40Z

Coverage remained the same at ?% when pulling 2ade84b on plone.app.discussion-moderation into de8614a on master.

tisto · 2019-12-11T21:47:05Z

@ksuess thank you for your contribution. The code looks fine to me. Though, could you please elaborate on how you ran into this issue? I would like to understand the rationale behind this change. Did you create a test layer that includes both plone.restapi and plone.app.discussion?

ale-rt

Not sure what is the need to to change the test but the method used is deprecated (to my knowledge).

ale-rt · 2019-12-11T22:28:06Z

src/plone/restapi/tests/test_services_querystring.py

@@ -93,7 +93,7 @@ def test_endpoint_inlines_vocabularies(self):
            u"published": {u"title": u"Published with accent \xe9 [published]"},
            u"visible": {u"title": u"Public draft [visible]"},
        }
-        self.assertEqual(expected_vocab_values, idx["values"])
+        self.assertDictContainsSubset(expected_vocab_values, idx["values"])


This is deprecated: https://docs.python.org/2/library/unittest.html#unittest.TestCase.assertDictContainsSubset

Thank you for reviewing. I'll fix this as soon as the main PR plone/plone.app.discussion#165 is reviewed and approved.

ksuess · 2019-12-12T08:40:21Z

@ksuess thank you for your contribution. The code looks fine to me. Though, could you please elaborate on how you ran into this issue? I would like to understand the rationale behind this change. Did you create a test layer that includes both plone.restapi and plone.app.discussion?

Comments moderation: Additional workflow with states pending, approved, rejected and spam plone/plone.app.discussion#165

Testing with
plone/plone.app.discussion#165
collective/plone.app.locales#285

plus
#843
plone/Products.CMFPlacefulWorkflow#30
plone/Products.CMFPlone#3002
for failing tests on additional workflow states.

https://github.com/plone/plone.app.discussion/pull/165
https://github.com/collective/plone.app.locales/pull/285
https://github.com/plone/plone.restapi/pull/843
https://github.com/plone/Products.CMFPlacefulWorkflow/pull/30
https://github.com/plone/Products.CMFPlone/pull/3002

tisto · 2019-12-12T15:29:47Z

Ideally, we would keep the plone.app.discussion workflows out of the Plone core testing layer. Though, I guess this is beyond the scope of this. If anybody wants to work on it this would be a worthwhile goal...

ksuess · 2020-01-11T11:55:29Z

Obsolete.
plone/plone.app.discussion#166 extends the existing review workflow instead of adding an additional workflow.

ksuess · 2020-01-13T08:25:47Z

Reopened pull request: tests need to take plone.app.discussion workflow stati into account, even for replaced pull request in plone.app.discussion plone/plone.app.discussion#166

mister-roboto · 2020-01-13T08:25:54Z

@ksuess thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

ale-rt · 2020-01-14T22:06:03Z

There seems to be a genuine failure on plone/app/discussion/tests/functional_test_comments.txt

plone.app.discussion introduces an additional workflow. This change takes the additional workflow states into account.

replace deprecated assertDictContainsSubset with assertLessEqual on dictionary items()

ale-rt · 2020-02-08T18:09:14Z

@jenkins-plone-org please run jobs

ale-rt

LGTM
IMO this can be merged also without the other PRs related to the ongoing work in plone.app.discussion.

mauritsvanrees · 2020-03-09T11:33:28Z

This indeed seems ready for merge, even without merging the related plone.app.discussion PR.
Jenkins 5.1 fails, but that job should not have been run in the first place: plone.restapi is not used there (the Jenkins error is the plone.restapi is not in sources.cfg).

@plone/restapi-team When convenient, can you merge?

jensens · 2020-03-13T15:25:05Z

IMO a minimal change which absolutely makes sense.

ale-rt requested changes Dec 11, 2019

View reviewed changes

ksuess mentioned this pull request Dec 12, 2019

Comments moderation: Additional workflow with states pending, approved, rejected and spam plone/plone.app.discussion#165

Closed

ksuess mentioned this pull request Jan 11, 2020

Extended Moderation of comments plone/plone.app.discussion#166

Merged

ksuess closed this Jan 11, 2020

ksuess reopened this Jan 13, 2020

ksuess force-pushed the plone.app.discussion-moderation branch from 398fab6 to c6d631d Compare January 13, 2020 08:38

ksuess requested a review from ale-rt January 14, 2020 09:00

ksuess force-pushed the plone.app.discussion-moderation branch from c6d631d to 8f3550c Compare January 23, 2020 10:27

ksuess added 4 commits February 4, 2020 10:26

Update test: plone.app.discussion

cbad98a

plone.app.discussion introduces an additional workflow. This change takes the additional workflow states into account.

Update test_services_querystring.py

44d832f

replace deprecated assertDictContainsSubset with assertLessEqual on dictionary items()

test if default workflows exist

3ea677f

Update 842.bugfix

1813e19

ksuess force-pushed the plone.app.discussion-moderation branch from 8f3550c to 1813e19 Compare February 4, 2020 09:26

ale-rt approved these changes Feb 8, 2020

View reviewed changes

jensens merged commit 93f6db5 into master Mar 13, 2020

jensens deleted the plone.app.discussion-moderation branch March 13, 2020 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update test: plone.app.discussion #843

Update test: plone.app.discussion #843

ksuess commented Dec 10, 2019

mister-roboto commented Dec 10, 2019

coveralls commented Dec 10, 2019 •

edited

Loading

coveralls commented Dec 10, 2019

coveralls commented Dec 10, 2019

tisto commented Dec 11, 2019

ale-rt left a comment

ale-rt Dec 11, 2019

ksuess Dec 12, 2019

ksuess commented Dec 12, 2019

tisto commented Dec 12, 2019

ksuess commented Jan 11, 2020

ksuess commented Jan 13, 2020

mister-roboto commented Jan 13, 2020

ale-rt commented Jan 14, 2020

ale-rt commented Feb 8, 2020

ale-rt left a comment

mauritsvanrees commented Mar 9, 2020

jensens commented Mar 13, 2020

Update test: plone.app.discussion #843

Update test: plone.app.discussion #843

Conversation

ksuess commented Dec 10, 2019

mister-roboto commented Dec 10, 2019

coveralls commented Dec 10, 2019 • edited Loading

coveralls commented Dec 10, 2019

coveralls commented Dec 10, 2019

tisto commented Dec 11, 2019

ale-rt left a comment

Choose a reason for hiding this comment

ale-rt Dec 11, 2019

Choose a reason for hiding this comment

ksuess Dec 12, 2019

Choose a reason for hiding this comment

ksuess commented Dec 12, 2019

tisto commented Dec 12, 2019

ksuess commented Jan 11, 2020

ksuess commented Jan 13, 2020

mister-roboto commented Jan 13, 2020

ale-rt commented Jan 14, 2020

ale-rt commented Feb 8, 2020

ale-rt left a comment

Choose a reason for hiding this comment

mauritsvanrees commented Mar 9, 2020

jensens commented Mar 13, 2020

coveralls commented Dec 10, 2019 •

edited

Loading