[cypress] Fail in case of SMTP problems with a clear message

[muhme]

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 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.

Trouble with snmp-tester port configuration was seen mutiple times, e.g. on configuring docker or on Ubuntu you need to open firewall with ufw allow 1025. May also be related to #42515 and #42516. Was discussed with @laoneo to check SNMP at the moment the snmp-tester is initialized. But that's only half the battle, because the SNMP configuration must work on Node 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

• before PR
  • run all system tests e.g. with npm run cypress:run to see they are running without errors
  • hack configuration.php with an unused, non-LISTEN port e.g. $smtpport = '6789'
  • run the five system test specs with 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'
  • see the ugly errors Cannot read properties of undefined
• after PR
  • run the five system test specs again and see the nice errors ... you may need to check the SMTP configuration
  • run all system tests e.g. with npm run cypress:run (including the initial Installation step which overwrites configuration.php with correct snmpport) to see they are running without errors

Actual result BEFORE applying this Pull Request

system tests fail in case of trouble with errors like:

  1) Test in frontend that the privacy request view
       can submit an information request of type export without a menu item:
     TypeError: Cannot read properties of undefined (reading 'sender')
      at Context.eval (webpack://joomla/./tests/System/integration/site/components/com_privacy/Request.cy.js:16:23)

Expected result AFTER applying this Pull Request

system tests fail in case of trouble with errors like:

  1) Test in frontend that the privacy request view
       can submit an information request of type export without a menu item:
     AssertionError: There are not two mails, you may need to check the SMTP configuration: expected 0 to equal 2
      at Context.eval (webpack://joomla/./tests/System/integration/site/components/com_privacy/Request.cy.js:15:103)

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

• after this PR is merged, please upmerge in 5.1-dev, 5.2-dev and 6.0-dev branches

What was already tested?

• the PR was exhausted tested with the following steps:

  1. clone dev branch
     • on Mac and Windows applied 3 files from PR 43466 (as not merged so far)
     • on 6.0-dev applied 1 file from PR 43400 (as not upmerged so far)
     • install and run full system tests npm run run:cypress w/o errors 111 specs / 436 tests
  2. 'hacked' $smtpport = '6789' as unused non-LISTEN port and run five system tests specs with 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'
     11 failing from 22 tests / 5 specs
     to see the ugly errors
  3. fetched this PR and run again the 5 specs to see the nice errors 11 failing from 22 tests / 5 specs
  4. run full system tests npm run run:cypress w/o errors (this is doing Web installation step first and overwrites configuration.php with snmpport 1025)

• 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 problem

• ignored two more test failures for the moment on 6.0-dev as they were also failing before the PR

[brianteeman]

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.

[muhme]

thank you for your comment and questions

From reading this my understanding is that you are saying all the cypress tests are sending mail using smtp

yes, and in node there is running the module smtp-tester; and smtp-tester in running smtp-server. During the system tests, an embedded SMTP server is therefore running on node. This is used to receive and check the mails

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.

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

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.

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 smtp_port is configured and be opened in firewall, if there is one

and yes, this needs to be documented, i will do that along with an architecture picture, a few words about smtp-tester and joomla-cypress and additional hints for working with the Cypress tests (ELECTRON_ENABLE_LOGGING=1, it.only(), defaultCommandTimeout) - just waiting for the last README.md PR to be merged

[laoneo]

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.

[muhme]

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.

• this test exists already in administration/com_config/Application.cy.js with can send a test mail
• and if you use an port that is already in use, as me web server port 8888, the overall test suite fails already with:

Running:  Application.cy.js                                                               (1 of 1)
Your configFile threw an error from: cypress.config.js

We stopped running your tests because your config file crashed.

Error: listen EADDRINUSE: address already in use :::8888
    at Server.setupListenHandle [as _listen2] (node:net:1872:16)
    at listenInCluster (node:net:1920:12)
    at Server.listen (node:net:2008:7)
    at SMTPServer.listen (/Users/hlu/Desktop/no_backup/improve-smtp-handling/node_modules/smtp-server/lib/smtp-server.js:104:28)

In my opinion, that's adequate, ok?

[brianteeman]

/me confused
IF administration/com_config/Application.cy.js runs successfully then why dont the other tests with smtp run successfully

[muhme]

/me confused IF administration/com_config/Application.cy.js runs successfully then why dont the other tests with smtp run successfully

If Application.cy.js runs successfully then you may have different configurations in cypress.config.js and configuration.php. As the test in Application.cy.js is the only one, that takes the configuration only from cypress.config.js and used this to fill the Joomla formular to test mail. All other tests are using PHP configuration.php. Could you please check configuration.php:

public $mailer = 'smtp';
public $smtphost = 'localhost'; # must be the same as smtp_host in cypress.config.js
public $smtpport = 1025; # must be the same as smtp_port in cypress.config.js

configuration.php is created by the first Installation.cy.js correctly with the values from cypress.config.js. But the read-only problem, fixed in 43465, prevented setting the configuration values in the past.

Good luck 🫰

[brianteeman]

That was it!!

Once the configuration.php was set to
public $mailer = 'smtp';
public $smtphost = 'localhost';
public $smtpport = 1025;

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

[laoneo]

I still don't think that we should add such kind of error messages as the tests should expect a working environment. When they fail, that because there is a bug in the code and not setup. So I'm against the explanation in all the tests and recommend to abort the tests when the SMTP connection fails.

[muhme]

I still don't think that we should add such kind of error messages as the tests should expect a working environment. When they fail, that because there is a bug in the code and not setup. So I'm against the explanation in all the tests and recommend to abort the tests when the SMTP connection fails.

there is already a test 'can send a test mail' that fails if SMTP connection fails, see #43492 (comment)

this PR is 'only' about with which message other tests fail, e.g. if one mail is expected and actual there is no mail receivid:

• actual: TypeError: Cannot read properties of undefined (reading 'body')
• with PR: AssertionError: There is not just one mail, you may need to check the SMTP configuration: expected 0 to equal 1

there are three different situaions – 0/1/2 mails expected and you can see the error messages compared in https://github.com/joomla/joomla-cms/pull/43492/files#r1611512411

[laoneo]

there is already a test 'can send a test mail' that fails if SMTP connection fails

This test tests settings in the configuration form and is not here to test the environment. When I do a test and I expect no mail, then I have to worry if the SMTP settings are correct? This is not right, in tests, we always need to assume that the environment works ok.

The expect code is good, no complain about this. But the message is for me not needed as when I write for some reasons a bug in the mailing API and mails are not sent anymore, it gives a wrong impression. Tests expect a certain environment and should test if the functionality works as expected and not if the environment is ok. Did you understand it now?