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

Factor out robotframework test dependency #674

Open
tisto opened this issue Oct 14, 2023 · 4 comments
Open

Factor out robotframework test dependency #674

tisto opened this issue Oct 14, 2023 · 4 comments

Comments

@tisto
Copy link
Member

tisto commented Oct 14, 2023

The plone.app.contenttypes test layer is kind of the default layer for testing with Plone. With Plone 6 (Volto, headless CMS, ...) newer parts of Plone do not require the robotframework test dependencies.

The Robot tests in p.a.contenttypes really only test Classic UI. Since end-to-end tests should always test the full stack, I'd recommend moving those tests either to CMFPlone (where they might have belonged in the first place from an architectural point of view) or to a new "ClassicUI" package.

The goal would be that packages like plone.rest(api) can rely on "plone.app.contenttypes [test] without fetching the robot framework test dependencies. Another option would be to introduce a "[robot]" test extra in p.a.contenttypes that would fetch robotframework (though, the latter does not feel right from an architectural point of view).

@davisagli
Copy link
Member

I think a classic UI package is the right place for these. CMFPlone is the common base for all frontends and should not include e2e tests for one specific frontend.

@tisto
Copy link
Member Author

tisto commented Oct 15, 2023

@davisagli I agree. Though, we have Robot tests for ClassicUI in CMFPlone as well.

@mauritsvanrees since the robotframework dependencies are causing troubles. What do you think about starting a Classic UI package and at first just moving the Classic UI RF tests there? This would be a non-breaking change and it would allow us to get started and gain experience.

@mauritsvanrees
Copy link
Member

A ClassicUI package would indeed be the best place. But in Plone 7 plone.app.contenttypes may be such a ClassicUI-only package: I think with plone.volto you do not use any of the ClassicUI content types, and it only imports a few interfaces from plone.app.contenttypes (plus some more in migration code). So maybe they can just stay here.

But yes, for Plone 6.1 it could be fine to move robot tests to a new package, maybe even a package that only contains tests, like plone.classicuirobottests. Just thinking out loud.

cc @ale-rt who started on https://github.com/plone/plone.classicui in February.

@tisto
Copy link
Member Author

tisto commented Oct 18, 2023

@mauritsvanrees interesting idea. We indeed have our own Document/News Item/Event content types:

https://github.com/plone/plone.volto/blob/26e5fc1ed495aea740a5687029c7f860f2132b87/src/plone/volto/content.py#L4

However, all the other content types (Image, File, Link, Folder) still come from p.a.contenttypes. This would mean that for Plone 7 we would have to write upgrade steps that migrate all content objects away from p.a.contenttypes.

We also use the p.a.contenttypes test layer and always assume that the functionality is there. Therefore it could require significant effort to move away from p.a.contenttypes.

I need to think about this. Maybe others want to chime in.

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

No branches or pull requests

3 participants