-
-
Notifications
You must be signed in to change notification settings - Fork 223
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
TASK: Speed up Behat tests #4750
Merged
mhsdesign
merged 3 commits into
neos:9.0
from
mhsdesign:task/speedupBehavioralTestByNinetyThreePercent
Nov 18, 2023
Merged
TASK: Speed up Behat tests #4750
mhsdesign
merged 3 commits into
neos:9.0
from
mhsdesign:task/speedupBehavioralTestByNinetyThreePercent
Nov 18, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bwaidelich
changed the title
Task: speedup behavioral test by ninety three percent
TASK: Speedup behat tests
Nov 17, 2023
With this change we setup the content repository only once for all tests `$contentRepository->setUp();`. The projections contents are cleared in each iteration via `$contentRepository->resetProjectionStates();` and this works quite well for the escr core tests. For tests using the `@flowEntities` annotation and database reset we must avoid to drop all the escr tables as otherwise we would need to setup the cr again. This will be fixed in neos/behat#37. Why are we going through so much hassle to (hackily) setup the cr only once? Because each projection calls `$schemaManager->createSchema()` twice when being setup and this slows down test enormously by 80%
Currently we run tests only in sync mode `CATCHUPTRIGGER_ENABLE_SYNCHRONOUS_OPTION=1` as the async mode is broken: neos#4612 I presume the `sleep(2)` is an artefact for the async tests to work at some earlier point.
mhsdesign
force-pushed
the
task/speedupBehavioralTestByNinetyThreePercent
branch
from
November 17, 2023 18:57
254fd17
to
0505bb2
Compare
mhsdesign
restored the
task/speedupBehavioralTestByNinetyThreePercent
branch
November 17, 2023 20:32
bwaidelich
approved these changes
Nov 18, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fantastic, thanks for taking care!
kdambekalns
approved these changes
Nov 18, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 by 👀
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During the refactoring of our testsuite via #4455 a performance optimization got lost.
Previously we only
setUp
a content repository once because the schema creation in the setup is super slow: #4762By preventing the projections to be resetup again, we gain a noticeable performance improvement (about 80%) of the whole e2e tests on the ci:
locally they run in like a minute.
To let tests in
Neos.Neos
profit from the same performance boost, we need to avoid the cr tables being dropped due to the@flowEntities
tag (see neos/behat#37 (comment)). This. is achieved via the adjustments in neos/behat#37 which will respect the configuredignoredTables
configuration.Additionally a
sleep(2);
statement which was run for every scenario was hidden in theNeos.Neos
tests.This was an early hotfix for our previously async running tests. Currently we run tests only in sync mode
CATCHUPTRIGGER_ENABLE_SYNCHRONOUS_OPTION=1
as the async mode is broken: #4612Upgrade instructions
Review instructions
Things and answers (collected from neos/behat#37 and slack):
Dont we have to truncate the tables every time so the projections are empty?
Can we safely empty the tables? Would that be the equivalent to
resetProjectionStates
?Shouldnt we make
setUp
just faster by caching thecreateSchema
?Related: #4388