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

[4.4] Use joomla-cypress 1.1.1 #43722

Merged
merged 27 commits into from
Aug 7, 2024
Merged

Conversation

muhme
Copy link
Contributor

@muhme muhme commented Jun 30, 2024

Summary of Changes

Revised after 27 commits on 7 August 2024

The goal is to use the NPM joomla-cypress version 1.1.1 for all four active joomla-cms branches. Actual 4.4-dev uses 0.0.16; and 5.1-dev, 5.2-dev and 6.0-dev are using 1.0.3.

The following changes have already been separated from this PR and have already been merged into 4.4-dev:

Also released is NPM package joomla-cypress as version 1.1.1. 😄

Change:

  • The joomla-cypress version is set to ^1.1.1, so - among other things - the check for PHP warnings is activated again.
  • The Cypress commands overwritten by System Tests for a faster session-based login have been removed as they are included in joomla-cypress version 1.1.1.
  • The note about overwritten commands in tests/System/README.md is deleted.

⚠️ Only NPM package joomla-cypress was updated, BUT there are more changes in package-lock.json. ⚠️

  • Reasons are
    • Dependend packages like cypress and cypress-file-upload
    • package.json and package-lock.json were not in-sync. For example in package.json the version is stated as '4.4.7' and in package-lock.json as '4.4.6'.

Successfully retested with all System Tests passed on 7 Agust 2024 after 27 commits:

  • on Intel macOS 14.5 Sonoma with PHP 8.3.9
  • on Intel Windows 11 Pro with Laragon and PHP 8.1.10
  • on Intel x86_64 Ubuntu 24.04 LTS Noble Numbat with PHP 8.3.6

Testing Instructions

  • Install (in using npm ci) and run System Tests w/o errors
  • Check version is 1.1.1 in node_modules/joomla-cypress/package.json
  • Check that the additional updates in the package-lock.json file had no effect.

Actual result BEFORE applying this Pull Request

  • System Tests is using joomla-cypress version 0.0.16 and a custom implementation of the faster session-based login
  • No checks of PHP warnings
  • System Tests suite is running without errors

Expected result AFTER applying this Pull Request

  • System Tests is using joomla-cypress actual version 1.1.1 and its session-based login
  • PHP warning checks are activated again
  • System Tests suite is running without errors

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

Up-Merging

@LadySolveig @bembelimen I will create a separate different PR for 5.1-dev

muhme added 7 commits June 7, 2024 06:47
Consider file permissions when writing configuration in system tests …
merge joomla/joomla-cms branch 4.4-dev
The joomla-cypress NPM package used has been updated to the latest version in order to be able to use one version in all active joomla-cms branches.
Other packages were also updated to the latest versions with the `npm update`.
The overwritten Cypress commands for faster login with session have been deleted, as they are already included in the new joomla-cypress version.
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.4-dev Unit/System Tests labels Jun 30, 2024
@hans2103
Copy link
Contributor

I have tested this item ✅ successfully on 9a66d94


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43722.

@richard67
Copy link
Member

@muhme Which command have you used to update the dependency? To me it seems this PR updates more dependencies than only joomla-cypress and it’s dependencies.

@muhme
Copy link
Contributor Author

muhme commented Jul 1, 2024

@muhme Which command have you used to update the dependency? To me it seems this PR updates more dependencies than only joomla-cypress and it’s dependencies.

@richard67 npm update to stay up-to-date with all dependencies. Should it changed to only update joomla-cypress and it’s dependencies?

@richard67
Copy link
Member

richard67 commented Jul 1, 2024

Should it changed to only update joomla-cypress and it’s dependencies?

@muhme Yes. We don’t do dependency updates with patch versions. And to update all dependencies without even mentioning it in the description of this PR would not be good anyway. @laoneo can explain and decide.

@alikon
Copy link
Contributor

alikon commented Jul 1, 2024

the #43726 should avoid the need for the hack in newsfeed and in the related_items module....

@muhme muhme marked this pull request as draft July 1, 2024 06:44
@muhme
Copy link
Contributor Author

muhme commented Jul 2, 2024

Set to DRAFT to:

@muhme
Copy link
Contributor Author

muhme commented Jul 2, 2024

And to update all dependencies without even mentioning it in the description of this PR would not be good anyway.

@richard67 Since the creation of this PR, the following statement has been included in the description:

⚠️ All other NPM packages are also updated with the npm update, see differences in package-lock.json.

The fact that all packages are updated is therefore explicitly named from my point of view and even provided with a visual eye-catcher. Also, the following instruction was added to the "Test Instructions" section since this PR was created:

  • Check that there were no effects from the updates in the NPM packages.

Could it be that you haven't read the description, or am I the one who's in the wrong movie? 😃

@richard67
Copy link
Member

@muhme Seems I need glasses 👓:-) You are right, description is ok, sorry for the noise :-)

@muhme muhme marked this pull request as ready for review July 9, 2024 15:29
@muhme
Copy link
Contributor Author

muhme commented Jul 9, 2024

Reverted to "Ready to merge", the description has been completely revised after all changes.

@laoneo
Copy link
Member

laoneo commented Jul 22, 2024

Are the changes in the View classes really needed?

@muhme
Copy link
Contributor Author

muhme commented Jul 22, 2024

Are the changes in the View classes really needed?

In 4.4-dev with joomla-cypress 0.0.16 the PHP message and warningas checks were disabled. With joomla-cypress 1.1.0 they are enabled and the System tests had 6 failures. At first there were workaround 'hacks' in the system tests to get the test suite running without errors. @alikon solved all system test 'hacks' by backporting and investigating. @alikon: Can you explain more in detail?

@laoneo
Copy link
Member

laoneo commented Jul 22, 2024

If these are bugs in the code, we should do them separate and then they need also two human tests.

@brianteeman
Copy link
Contributor

Golden rule - solve one issue with one pull request

@muhme muhme marked this pull request as draft July 22, 2024 17:00
@muhme muhme changed the title Update to joomla-cypress 1.1.0 Update to joomla-cypress 1.1.1 Aug 4, 2024
@muhme muhme changed the title Update to joomla-cypress 1.1.1 [4.4] Use joomla-cypress 1.1.1 Aug 4, 2024
@laoneo
Copy link
Member

laoneo commented Aug 6, 2024

As the other pr's are merged, can this one be changed to ready?

@muhme
Copy link
Contributor Author

muhme commented Aug 6, 2024

As the other pr's are merged, can this one be changed to ready?

No, it's intentionally set to 'Draft'. I would prefer to wait for the release of joomla-cypress 1.1.1 and then update this PR, set it on 'Ready to Merge' and create separate PR for all other branches, ok?

@muhme muhme marked this pull request as ready for review August 7, 2024 13:29
@muhme
Copy link
Contributor Author

muhme commented Aug 7, 2024

@laoneo Set to ready-for-review, as all preconditions have been fulfilled and the PR has been adjusted and pre-tested

@alikon Could you please test?

@muhme muhme mentioned this pull request Aug 7, 2024
4 tasks
@laoneo laoneo merged commit 46c3bc0 into joomla:4.4-dev Aug 7, 2024
3 checks passed
@laoneo
Copy link
Member

laoneo commented Aug 7, 2024

Thanks!

@laoneo laoneo added this to the Joomla! 4.4.7 milestone Aug 7, 2024
@muhme muhme deleted the joomla-cypress-update branch August 7, 2024 16:45
dgrammatiko pushed a commit to dgrammatiko/joomla-cms that referenced this pull request Aug 11, 2024
* [cypress] Updated to joomla-cypress 1.1.0

The joomla-cypress NPM package used has been updated to the latest version in order to be able to use one version in all active joomla-cms branches.
Other packages were also updated to the latest versions with the `npm update`.
The overwritten Cypress commands for faster login with session have been deleted, as they are already included in the new joomla-cypress version.

* Hack to prevent deprecated in mod_related_items

* 3 more files with workarounds

* no hack for modules

* avoid hacks

* avoid hacks

* featured

* featured

* module

* nomore

* nomore

* Fix missing apostrophe

* NPM Update only joomla-cypress

npm install joomla-cypress@1.1.0

* Update tests/System/README.md

Removed notice on overridden joomla-cypress commands

* Fixing lint:testjs errors

* Undo the changes extracted to a separate PRs

* npm update joomla-cypress 1.1.1

---------

Co-authored-by: Nicola Galgano <optimus4joomla@gmail.com>
Co-authored-by: Allon Moritz <allon.moritz@digital-peak.com>
dgrammatiko pushed a commit to dgrammatiko/joomla-cms that referenced this pull request Aug 11, 2024
* [cypress] Updated to joomla-cypress 1.1.0

The joomla-cypress NPM package used has been updated to the latest version in order to be able to use one version in all active joomla-cms branches.
Other packages were also updated to the latest versions with the `npm update`.
The overwritten Cypress commands for faster login with session have been deleted, as they are already included in the new joomla-cypress version.

* Hack to prevent deprecated in mod_related_items

* 3 more files with workarounds

* no hack for modules

* avoid hacks

* avoid hacks

* featured

* featured

* module

* nomore

* nomore

* Fix missing apostrophe

* NPM Update only joomla-cypress

npm install joomla-cypress@1.1.0

* Update tests/System/README.md

Removed notice on overridden joomla-cypress commands

* Fixing lint:testjs errors

* Undo the changes extracted to a separate PRs

* npm update joomla-cypress 1.1.1

---------

Co-authored-by: Nicola Galgano <optimus4joomla@gmail.com>
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
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.4-dev Unit/System Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants