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 #74

Merged
merged 6 commits into from
Mar 13, 2023

Conversation

gforcada
Copy link
Sponsor Member

@gforcada gforcada commented Mar 12, 2023

@mauritsvanrees should we keep all the tox environments? I see they are being run on GHA, so I left them 🍀

@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-2827ab91 branch from 1f24ac2 to f249609 Compare March 12, 2023 22:08
@gforcada gforcada force-pushed the config-with-default-template-2827ab91 branch from f249609 to 7c66116 Compare March 12, 2023 23:34
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.

should we keep all the tox environments? I see they are being run on GHA, so I left them 🍀

Yes, leave them please, I want to keep testing on 5.2 Py 3 for this package. Same for plone.namedfile.

The mxdev stuff in the existing environments is currently not needed, but that may change in the future if we need to test checkouts or different versions of plone.scale and plone.namedfile together, as we needed for a part of last year.

I approve, but Jenkins tests still need to run, and I see that currently core Jenkins already fails with a problem in plone.z3cform.

@gforcada
Copy link
Sponsor Member Author

I'm reverting plone.z3cform checkout, let's see if that is enough.

Meanwhile PR jenkins jobs do work 🤔 the magic -j4 might not be that fail safe? 🤔

@mauritsvanrees
Copy link
Sponsor Member

Meanwhile PR jenkins jobs do work 🤔 the magic -j4 might not be that fail safe? 🤔

With four threads, each thread has less layers that it needs to run together, which means there is less chance for layers to badly influence each other. We could let the core jobs run with j4 as well. But it would hide some bad layers.

It may be good to have one PR job run without j4, so we have early warning. But that makes the Jenkins setup harder.

@mauritsvanrees mauritsvanrees merged commit 4eeb09a into master Mar 13, 2023
@mauritsvanrees mauritsvanrees deleted the config-with-default-template-2827ab91 branch March 13, 2023 23:34
@gforcada
Copy link
Sponsor Member Author

With j4 we have speed and that's good, we could have a weekly/daily run in sequence to catch these layer setups...

Although is there an heuristic to know if a layer is broken, if I recall correctly we added a check in plone.testing for integration layers that if someone tries to make a transaction.commit() it aborts with a message.

Is there something else we could do the ensure we notice at runtime that something might be broken? 🤔

@mauritsvanrees
Copy link
Sponsor Member

In the plone.app.linkintegrity case, I was unintentionally adding content in setup or setupZope but did not create a tearDown method to clean up. I think that is hard to detect, except by simply seeing that some tests are failing.

What I remember from last year when debugging a similar problem at a client, was that it is hard for zope.testrunner (and presumably also pytest) to know which layers to tear down when. Consider this simplified example:

class PackageLayerOne(PloneSandboxLayer):
   ...

PACKAGE_ONE_FIXTURE = PackageLayerOne()
PACKAGE_ONE_INTEGRATION = layers.IntegrationTesting(bases=PACKAGE_ONE_FIXTURE)

class PackageLayerTwo(PloneSandboxLayer):
   ...

PACKAGE_TWO_FIXTURE = PackageLayerTwo()
PACKAGE_TWO_INTEGRATION = layers.IntegrationTesting(bases=PACKAGE_TWO_FIXTURE)

What is the best way to run these layers?

  1. Run all package 1 layers, then all package two layers?
  2. Run all unit tests with the base layers, then integration tests?

With the first, you get something like this:

  • Setup package layer 1.
    • Setup integration testing layer.
    • Teardown integration testing layer.
  • Teardown package layer 1.
  • Setup package layer 2.
    • Setup integration testing layer.
    • Teardown integration testing layer.
  • Teardown package layer 2.

With the second it is like this:

  • Setup package layer 1.
  • Teardown package layer 1.
  • Setup package layer 2.
  • Teardown package layer 2.
  • Setup integration testing layer.
    • Setup package layer 1.
    • Teardown package layer 1.
    • Setup package layer 2.
    • Teardown package layer 2.
  • Teardown integration testing layer.
  • Setup functional testing layer.
    • Setup package layer 1.
    • Teardown package layer 1.
    • Setup package layer 2.
    • Teardown package layer 2.
  • Teardown functional testing layer.

In both scenarios, layers are setup and torn down multiple times.

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