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

[5.2] add test for site router (sef) #44253

Merged
merged 3 commits into from
Oct 30, 2024

Conversation

heelc29
Copy link
Contributor

@heelc29 heelc29 commented Oct 14, 2024

Summary of Changes

add tests for site router (sef) and should cover:

Testing Instructions

drone
code review

Actual result BEFORE applying this Pull Request

no sef tests

Expected result AFTER applying this Pull Request

test coverage

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed
  • No documentation changes for manual.joomla.org needed

@heelc29 heelc29 force-pushed the 5.2/tests/site-router-sef branch 6 times, most recently from 912981f to 8f83686 Compare October 14, 2024 19:38
@laoneo
Copy link
Member

laoneo commented Oct 15, 2024

You can rename the htaccess.txt in beforeEach to .htaccess and in afterEach back to .txt.

@laoneo laoneo self-assigned this Oct 15, 2024
@heelc29
Copy link
Contributor Author

heelc29 commented Oct 15, 2024

You can rename the htaccess.txt in beforeEach to .htaccess and in afterEach back to .txt.

At the moment I'm struggle a bit with copying/renaming the file.

@heelc29 heelc29 force-pushed the 5.2/tests/site-router-sef branch 5 times, most recently from b1fd7fc to c81fd1b Compare October 15, 2024 18:38
@heelc29
Copy link
Contributor Author

heelc29 commented Oct 15, 2024

It looks like mod_rewrite is not enabled in docker image.

@heelc29 heelc29 force-pushed the 5.2/tests/site-router-sef branch from 33a9baf to 9022974 Compare October 28, 2024 08:35
@heelc29 heelc29 marked this pull request as ready for review October 28, 2024 08:35
@heelc29
Copy link
Contributor Author

heelc29 commented Oct 28, 2024

Ready for review

@laoneo laoneo merged commit 0662f56 into joomla:5.2-dev Oct 30, 2024
3 checks passed
@laoneo
Copy link
Member

laoneo commented Oct 30, 2024

@heelc29 I contacted you on matermost, if you have time, would be great if you can respond.

Anyway, good work!

@laoneo laoneo added this to the Joomla! 5.2.1 milestone Oct 30, 2024
muhme added a commit to muhme/joomla-branches-tester that referenced this pull request Dec 13, 2024
To be able to test plugins/system/sef/SefPlugin.cy.js,
introduced with joomla/joomla-cms#44253
and first time using Joomla command line client.

Not supporting Cypress GUI
muhme added a commit to muhme/joomla-cms that referenced this pull request Dec 22, 2024
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
```
@heelc29 heelc29 deleted the 5.2/tests/site-router-sef branch January 19, 2025 18:24
laoneo added a commit that referenced this pull request Jan 25, 2025
… Client Tool (#44656)

* [Cypress] Remove Joomla command line dependency

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](#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
```

* writeRelativeFile() header adapted return Promise

Thank @heekc29 for the hint

* Delete cy.log in custom command

---------

Co-authored-by: Allon Moritz <allon.moritz@digital-peak.com>
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