-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[cypress] Fail in case of SMTP problems with a clear message #43492
base: 4.4-dev
Are you sure you want to change the base?
Conversation
If the array `mail[]` is empty width length 0, the line `cy.wrap(mails).should('have.lengthOf', 1);` does not cause the test to stop in this case as the Cypress assertion inside the `.then()` has an asynchronous nature. The effect is that the test run continues and the following access to the mail fails with with a somehow incomprehensible message e.g. `TypeError: Cannot read properties of undefined (reading 'body')` Replaced it with an `expect` statement, which immediately triggers an error, and added an individual message that the SNMP configuration may need to be checked. Added two more places.
From reading this my understanding is that you are saying all the cypress tests are sending mail using smtp and the tests that I reported as failing were because my test server was not configured to use smtp and the changes here are so that the failure message is clear that it is failing because of smtp errors. I hope I understood that correctly. If I have then I would make changes to the cypress readme etc to make it clear that the tests require a working smtp server. This was obvioulsy not apparent before hence the bug reports and as Joomla would normaly be configured to use phpmail or sendmail or smtp it explains why we had the "it works for me on my server" scenario. Even with this PR I am sure it could happen again if this requirement was not documented more clearly. |
tests/System/integration/administrator/components/com_config/Application.cy.js
Show resolved
Hide resolved
thank you for your comment and questions
yes, and in node there is running the module
it would be interesting if you repeat the tests as described in this PR in your environment, we can also do a screen session together, i am now a little familiar with Laragon env
no, a working SMTP server is not required for the system tests, as this is embedded in the system test suite. important is that a free, unused and yes, this needs to be documented, i will do that along with an architecture picture, a few words about |
Thanks for the pr' it is looking good. The only issue I have is that the SMTP server is a hard requirement for the test suite. So if it is not working on startup no test running should start at all as the environment is not properly setup. So I'm for an additiional check in the SMTP server setup function if the server is reachable. |
In my opinion, that's adequate, ok? |
/me confused |
If
Good luck 🫰 |
That was it!! Once the configuration.php was set to Then the tests worked - without the need of this pull request. Therefore I would suggest that this PR could be improved by adding that smtp configuration info to the readme |
Summary of Changes
Improved Cypress system test failure messages in case of problems with SMTP.
If the array
mail[]
is empty width length 0, the linecy.wrap(mails).should('have.lengthOf', 1);
does not cause the test to stop, in this case, as the Cypress assertion inside the
.then()
has an asynchronous nature.The effect is that the test run continues and the following access to the mail fails with with a somehow incomprehensible message e.g.
TypeError: Cannot read properties of undefined (reading 'body')
Replaced it with an
expect
statement, which immediately triggers an error, and added an individual message that the SNMP configuration may need to be checked. Added two more places.Trouble with
snmp-tester
port configuration was seen mutiple times, e.g. on configuring docker or on Ubuntu you need to open firewall withufw allow 1025
. May also be related to #42515 and #42516. Was discussed with @laoneo to check SNMP at the moment thesnmp-tester
is initialized. But that's only half the battle, because the SNMP configuration must work onNode
and in the PHP-code of the server. This is why an individual test error message is used with a reference to the SNMP settings. And overall, a test must fail immediately if a mail is expected and is not there.Testing Instructions
npm run cypress:run
to see they are running without errorsconfiguration.php
with an unused, non-LISTEN port e.g. $smtpport = '6789'smtp-tester
usage:npm run cypress:run -- --spec 'tests/System/integration/administrator/components/com_config/Application.cy.js,tests/System/integration/site/components/com_users/Remind.cy.js,tests/System/integration/site/components/com_privacy/Request.cy.js,tests/System/integration/site/components/com_users/Reset.cy.js,tests/System/integration/api/com_contact/Contacts.cy.js'
Cannot read properties of undefined
... you may need to check the SMTP configuration
npm run cypress:run
(including the initial Installation step which overwritesconfiguration.php
with correctsnmpport
) to see they are running without errorsActual result BEFORE applying this Pull Request
system tests fail in case of trouble with errors like:
Expected result AFTER applying this Pull Request
system tests fail in case of trouble with errors like:
Link to documentations
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed
Upmerge
What was already tested?
the PR was exhausted tested with the following steps:
npm run cypress:run -- --spec 'tests/System/integration/administrator/components/com_config/Application.cy.js,tests/System/integration/site/components/com_users/Remind.cy.js,tests/System/integration/site/components/com_privacy/Request.cy.js,tests/System/integration/site/components/com_users/Reset.cy.js,tests/System/integration/api/com_contact/Contacts.cy.js'
11 failing from 22 tests / 5 specs
to see the ugly errors
did these four steps on the following platforms:
a) local with branch 4.4-dev on Intel macOS 14.4.1 / Apache / MySQL
b) local with branch 4.4-dev on Windows 11 Pro / Laragon
c) on docker based installation
. with branch 4.4-dev 435 tests (w/o Installation step)
. with branch 5.1-dev 440 tests (w/o Installation step)
. with branch 5.2-dev 440 tests (w/o Installation step)
. with branch 6.0-dev 440 tests (w/o Installation step)
ignored test failure
api/com_menus/SiteMenuItems.cy.js
-can create a site menu item
Save failed with the following error: Joomla\\Component\\Menus\\Administrator\\Table\\MenuTable::_getNode(1, id) failed."
for the moment, assuming it is a mutliple test run problemignored two more test failures for the moment on 6.0-dev as they were also failing before the PR