-
Notifications
You must be signed in to change notification settings - Fork 91
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
Find opportunities to speed up MFTF tests #652
Comments
Thanks @filmaj that does sum it up pretty well, I've been having a bit of a dig to try to understand more as to why it takes so long to run on my local system and I can at least explain some of it: So the output from a local run of AdminAdobeStockImagePreviewSameModelSeeMoreTest is as follows:
So it takes almost 6 minutes to run. What I noticed is that for each of the ramp up and ramp down bits, after each click there is a "wait for page load 30" and this seemed to be where it was hanging around the most for me. I had a dig a bit more into why this was happening and found some interesting stuff. So for example, take the "openViewBookmarks", which is part of the core magento action group "resetAdminDataGridToDefaultView" defined in "app/code/Magento/Ui/Test/Mftf/ActionGroup/AdminDataGridFilterActionGroup.xml":
The interesting bit is that there are no page load waits defined there there, but in my test run, they definitely happen! Turns out it's down to the section definition. See "app/code/Magento/Ui/Test/Mftf/Section/AdminDataGridHeaderSection.xml":
Both "bookmarkToggle" and bookmarkOption have a timeout value of 30, which upon doing a bit of digging, led me to the "insertWaits" function in "vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Test/Util/ActionMergeUtil.php". Which has the following code:
This means that when generating the tests, it sees the 30s timeout in the section definition and adds a wait in there. I have no idea, however, why this would affect me, but not others. Perhaps theres an MFTF/selenium setting that makes it ignore timeouts? For context, I'm running Selenium standalone with the following command: java -jar selenium-server-standalone-3.141.59.jar With the chrome driver for mac 64 bit Possible suggestion if it's not just an isolated issue: perhaps we do away with relying on these core ramp-up and ramp-downs and write our own? We can use the "waitForElementVisible" to wait for selected elements to appear on the page rather than a defined time period? See https://devdocs.magento.com/mftf/docs/test/actions.html#waitforelementvisible) |
Further note: I changed the timeouts in AdminDataGridHeaderSection.xml to 5, and this reduced the test to the following:
I'm going to set up the project on a different PC (much more powerful, and ubuntu instead of Mac) later in the week, and will report back with the stats I get there :) |
One last comment for now (sorry for the spam!) I noticed this output from the selenium command:
So I updated my chromedriver to the correct version for my chrome version, but it didn't make a difference, other than removing that warning :( selenium.jar output was:
|
Hi @filmaj @semajeg. Thank you! |
Here's something which took forever on my machine: TWELVE minutes, to be precise. Maybe since this is a long test, but I see lots of
This is how I start the Selenium server: (version is shown here)
Maybe this could help, @rogyar. Thanks. |
@rogyar the command was:
also tried it without the --remove , didn't make any difference |
@semajeg @drpayyne could you do me a favour and also attach or link to the full output of your On first glance, the MFTF output is the same as mine, but my setup does not get stuck on the 'optionally wait for 30 seconds' bits you pointed out, @semajeg, so... |
One thing I noticed is that both of you have Can you check whether you see any JS errors when you open the Media Gallery on your instances by checking in your browser developer console? |
@filmaj Regarding the JS error, this is my console when I open the Media Gallery. Error source: |
@filmaj, regarding the java output for the test mentioned above (the 12 minute one!):
|
Can you tweak the capabilities passed to chromedriver to enable chromedriver to do verbose logging and logging to a specific path? In my (embedded MFTF) setup, I had to edit EDIT: wait, no, those are chrome options, not chromedriver options. Hmm... let me look around on how to do this. |
Try running:
... and then attach or link to |
Had an opportunity to dig into this a bit deeper with @drpayyne. Some interesting findings. First, the Second was going back to the JavaScript exception reported in the MFTF output ( So the working hypothesis at this point is that it is this JavaScript exception that is the root cause of the slowdowns, because it causes jQuery to report itself as still working on AJAX requests, which then causes each We need to get to the bottom of this exception. So, three requests for you @drpayyne and @semajeg:
That should help me dig deeper to understand where in the client side code this issue is happening and why. Thanks! |
Thanks for the detailed review and assistance, @filmaj! Here are my environment logs and details:
and browser.js is Environment is Ubuntu 18.04.3 LTS and Chromium Version 77.0.3865.90 (Official Build) Built on Ubuntu, running on Ubuntu 18.04 (64-bit). Magento version is Hope this helps, thank you! |
Thanks for all the info @filmaj! I think you may have hit the nail on the head with the JS error being the problem! I'm getting exactly the same error as @drpayyne, so I'm getting the error on line 92 of /lib/web/mage/adminhtml/browser.js too, the code on that line is:
I think the reason for the JS error is that the onclick for the "Insert image..." button doesn't provide any "options" to the openDialog function (I think the html for it gets generated in So options is undefined, hence the error after the ajax to load the slideout modal. I don't that is unexpected behaviour, but I think this is a actually bug in core Magento introduced fairly recently in that js file as that line was added that line changed 2 months ago, see line 92 in https://github.com/magento/magento2/blame/2.3-develop/lib/web/mage/adminhtml/browser.js Looks like this PR added the line magento/magento2#24333 It's likely you're not seeing the error because you have an older copy of the repo. I'm running a clone of the 2.3-develop branch from just before MLEU (so Oct 20th ish) which included the PR that introduced this. As a test, I changed my local version of browser.js to wrap that line in an if statement:
I then reran the test I was running before, and it runs in 1.35 minutes (down from 5.69). Tbh, a lot of time is now in the setup before selenium even gets going, so I think I've hit the limit on what my old mac can do, but it's more respectable than almost 6 mins! So the JS error must be screwing around with chromedrive/selenium as you've said. For extra context, I'm running the following: OS: MacOS 10.13.6 |
Thanks for the info @semajeg. Indeed I am running the tagged 2.3.3 version of magento core, and the setting of targetElementId is not present in browser.js in my instance. So, at least we have a workaround to speed up the tests: be defensive when attempting to use the I'm gonna queue up a task to see if I can reproduce this issue with latest magento locally, and if so, file an issue on magento2 core. |
BTW can confirm that when moving to latest develop of magento2 core, I have the JS exception causing the same slowdowns in the tests. @semajeg's workaround worked great, thankfully. |
@filmaj should this ticket be closed as the fix is delivered to magento2 core with magento/magento2#25556 ? |
Yep, good catch! |
Is your feature request related to a problem? Please describe.
It was discussed today in the community call with @drpayyne and @semajeg that the MFTF tests, when running locally, seem to have different performance profiles and bottlenecks depending on
each developer's system. Given MFTF is based on Selenium, this is not entirely surprising, but e.g. on my system most tests run in under one minute (the entire suite of 26 tests runs in ~25 mins for me), whereas others have described 4-5 minute runs for certain tests.
One area of opportunity seems to be a common teardown clause for many tests: resetting the Adobe Stock Search grid filters. This is called via
resetAdminDataGridToDefaultView
- which actually is not part of this project (on my instance it is present underapp/code/Magento/Ui/Test/Mftf/ActionGroup/AdminDataGridFilterActionGroup.xml
). This seems to take 2-3 minutes to complete on certain contributors' machines: @drpayyne @semajeg. Gentlemen, do I have the details there correct? Any further details you could provide? I think it would be helpful to get the fullmftf run:test
output as well as saving the Selenium server output and attaching it to this issue. I have experience with Selenium (I used to work at a Selenium SaaS) so I will take a look and see if anything in those logs stand out.Another one seems to be a common setup clause for many tests: loading the Media Gallery and then clicking into the Adobe Stock search grid. I recently modified the open media gallery ActionGroup to have two
waitForPage
statements (I provided comments to clarify why), but perhaps this is not optimal.Describe the solution you'd like
Would be great to have sub-1-minute tests for all contributors.
Describe alternatives you've considered
--headless
option to the chrome capabilities options list at the bottom of thedev/tests/acceptance/tests/function.suite.yml
file. My usual approach is to use headless when running the entire suite of tests (i.e.vendor/bin/mftf run:group adobe_stock_integration_suite --remove -vvv
) and remove that option when running individual tests when debugging failures (i.e.vendor/bin/mftf run:test AdminAdobeStockImagePreviewKeywordsSearchTest --remove
).The text was updated successfully, but these errors were encountered: