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

[Cypress] Remove Joomla command line client tool dependency #44656

Open
wants to merge 2 commits into
base: 5.2-dev
Choose a base branch
from

Conversation

muhme
Copy link
Contributor

@muhme muhme commented Dec 22, 2024

Summary of Changes

Refactoring of the Cypress test for SEF to replace the dependency on the Joomla command line client tool with Cypress custom commands.

With the SefPlugin.cy.js Cypress test, the Joomla command line client tool was added to the Joomla System Tests for the first time. This is an additional complexity and dependency. Especially if the test environment is containerised and Cypress container has no PHP installed. And PHP version needs to be 8.1 and additional modules like php-simplexml are needed. And with the Docker access rights, configuration.php must be opened for writing (chmod 644). And then there is the locally running Cypress GUI to be configured...

Therefore switched from using the Joomla command line client tool to using the existing config_setParameter Cypress custom command. The original creator of 44253 could not know as it is not documented (will be solved with another PR #44660, together with the description for configuring the now required enable mod_rewrite and AllowOverride All).

Since sometimes the first SEF test failed when the entire Joomla System Tests was performed after refactoring, the following was implemented:

  • The operations of the writeRelativeFile Cypress task are wrapped in a Promise to ensure that they are only resolved when everything is done
  • The config_setParameter Cypress custom command to return a Cypress chainable
  • Chaining has been used everywhere in SefPlugin.cy.js

The Cypress best practice of using beforeEach is used to ensure that the tests can be run independently (this is still needed for other places in the Joomla system tests - another PR).

As a small side effect the SEF test spec with 9 tests runs faster, e.g. 3 instead of 18 seconds before.

Testing Instructions

  1. Tested with own target Joomla 5.2-dev three times the single test spec on:
  • Intel macOS 15.2 using Apache/MariaDB
  • Apple Silicon macOS 15.2 using Apache/MariaDB
  • Windows 11 24H2 using Laragon/Apache/MySQL
npx cypress run --spec tests/System/integration/plugins/system/sef/SefPlugin.cy.js

And once the overall test suite:

npm run cypress:run
  1. Tested with JBT in creating Joomla 5.2-dev instance with this PR:
scripts/create 52 joomla-cms-44656

Three times the single test spec on:

  • Intel macOS 15.2
  • Apple Silicon macOS 15.2
  • Windows 11 24H2 WSL2 Ubuntu
scripts/test system plugins/system/sef/SefPlugin.cy.js

And once the overall test suite:

scripts/test system

Actual result BEFORE applying this Pull Request

Joomla System Tests require Joomla command line client

Expected result AFTER applying this Pull Request

Joomla System Tests doesn't require Joomla command line client

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

  • Related PR for adopting Joomla System Tests GitHub will follow

With the `SefPlugin.cy.js` test specification, the Joomla command line client tool was added to the Joomla system tests for the first time. This is an additional complexity and dependency. Especially if the test environment is containerised and Cypress container has no PHP installed.  And PHP version needs to be 8.1 and additional modules like `php-simplexml` are needed. And with the Docker access rights, `configuration.php` must be opened for writing (`chmod 644`).
And then there is the locally running Cypress GUI to be configured...

Therefore switched from using the Joomla command line client tool to using the existing `config_setParameter` Cypress command. The original creator of [44253](joomla#44253) could not know as
it is not documented (will be solved with another PR and together with enabling `mod_rewrite` and the `AllowOverride All` needed configuration).

Since sometimes the first SEF test failed when the entire Joomla System Tests was performed, the following was implemented:
* The operations of the `writeRelativeFile` Cypress task are wrapped in a Promise to ensure that they are only resolved when everything is done
* The `config_setParameter` Cypress custom command to return a Cypress chainable
* Chaining has been implemented everywhere in `SefPlugin.cy.js`

For this test spec the Cypress best practice to use `beforeEach` to ensure tests is added to
run independently from one another (and needed for other places in the Joomla System Tests – one more PR).

As a small side effect the SEF test spec with 9 tests runs faster, e.g. 3 instead of 18 seconds before.

Tested with own target Joomla 5.2-dev three times the single test spec on:
* Intel macOS 15.2 using Apache/MariaDB
* Apple Silicon macOS 15.2 using Apache/MariaDB
* Windows 11 24H2 using Laragon/Apache/MySQL
```
npx cypress run --spec tests/System/integration/plugins/system/sef/SefPlugin.cy.js
```
And once the overall test suite:
```
npm run cypress:run
```
@muhme muhme marked this pull request as ready for review December 22, 2024 06:25
@muhme
Copy link
Contributor Author

muhme commented Dec 22, 2024

Hey @heelc29 and @alikon could you please have a look and, if possible, test this? Thanks in advance! 😄

@muhme muhme requested a review from laoneo December 22, 2024 06:28
@muhme muhme changed the title [Cypress] Remove Joomla command line dependency [Cypress] Remove Joomla command line client tool dependency Dec 22, 2024
@laoneo laoneo self-assigned this Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants