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] Using NPM Module 'pg' for 'postgres' #44084

Merged
merged 4 commits into from
Oct 23, 2024

Conversation

muhme
Copy link
Contributor

@muhme muhme commented Sep 16, 2024

Last Edited: 24 September 2024: Finished Testing

Summary of Changes

Replace postgres with pg NPM module for Cypress System Tests:

  • IPv6 addresses are working with MySQL and MariaDB, but it appears that IPv6 support is not working properly with the postgres NPM module.
    . Getting error e.g. "getaddrinfo ENOTFOUND fd00" using database host fd00::6
    . No solvable in using [fd00::6], getting error "getaddrinfo ENOTFOUND [fd00"
    . looks like error is in how postgres module uses node
  • And pg is the official PostgreSQL client for Node.js, which is used ten times more frequently.
  • Should not have much impact as it's 'only' used in the Cypress System Tests.

👉 This change does not fix the rarely occurring PostgreSQL duplicated keys error as well. While this issue has been reduced by joomla-cms#43924, it still happens occasionally with using postgres, as well as with pg module.

A side-effect of adding the new module is that the version is updated to 4.4.9 in the package-lock.json file.
And pg has more dependencies.

Testing Instructions

One option for testing is using JBT >= 1.0.24 (which includes currently open joomla-projects/joomla-cypress#33, joomla-projects/joomla-cypress#36 and #43968):

scripts/create 44 IPv6 pgsql
scripts/patchtester 44
  1. System Tests with module 'postgres' and IPv6 hostname to measure time before
    • scripts/test 44 system
      • using db_host jbt_pg
      • using upto 10 servers sessions in total
      • 🟥 one, not reproducable failure seen: > duplicate key value violates unique constraint "jos44_users_pkey"
      • ✅ running 119 specs in 3:18 Cypress netto, 5:39 script brutto
  2. System Tests with NPM module 'postgress' and IPv6 address to see failure
    • find out IPv6 address: docker exec jbt_44 ping jbt_pg
    • changing branch_44/cypress.config.mjs
      • db_host: 'fd00::5'
    • Joomla installation (using Web Installer with only PHP) is running successful
      • scripts/test 44 system novnc install/Installation.cy.js
      • Checked: administrator | Global Configuration | Server | Database Host: fd00::5
    • System Tests, using NPM module 'postgres' with the custom database commands fail:
      • scripts/test 44 system novnc
        • 🟥 test specs are failing with "getaddrinfo ENOTFOUND fd00"


  1. System Tests to verify change with NPM module pg and IPv6 address
    • in backend | Patch Tester 'Apply Patch' this PR 44084
    • docker exec jbt_44 npm ci
    • scripts/test 44 system
      • using upto 11 servers sessions, checked with pgAdmin
      • ✅ running 119 specs in 3:11 Cypress netto, 5:31 script brutto time
  2. System Tests to verify change with NPM module pg and hostname
    • scripts/create 44 recreate pgsql
    • Checked: backend | Global Configuration | Server | Database Host: jbt_pg
    • scripts/test 44 system
      • using upto 11 servers sessions in total
      • 🟥 one, not reproducable error > Postgres query failed: duplicate key value violates unique constraint "jos44_users_pkey"
      • ✅ 119 specs passed successfully 3:19 Cypress netto, 5:40 script brutto time
  3. System Tests to verify change with NPM module pg , IPv6 address and non-default port number
    • We need to start with this PR to ensure the additional patches are installed by JBT and not overwritten later by npm install
      • scripts/create 44 IPv6 pgsql https://github.com/muhme/joomla-cms:44-pg-for-postgres
    • We are running socat on Cypress container, listen on :4711 and forward requests to PostgreSQL:
    • Install socat in Cypress container:
      • docker exec jbt_cypress bash -c 'apt-get update && apt-get upgrade -y && apt-get install socat -y'
    • Find out Cypress and PostgresSQL container IPv6 addresses:
      • docker exec jbt_44 ping jbt_cypress -> in my case fd00::8
      • docker exec jbt_44 ping jbt_pg -> in my case fd00::6
    • Changing branch_44/cypress.config.mjs
      • 👉 In cypress.config.mjs we are using host fd00::8 and port 4711 separate and in Joomla backend configuration.php we are using combined [fd00::8]:4711
      • `db_host: 'fd00::8',
      • `db_port: '4711',
    • Running socat in a separate terminal window
      • docker exec jbt_cypress socat 'TCP6-LISTEN:4711,fork,reuseaddr' 'TCP6:[fd00::6]:5432'
    • Verifying setup with client tool
      • docker exec -it jbt_pg bash -c "PGPASSWORD=root psql -h fd00::8 -p 4711 -U root -d postgres"
    • Running System Tests
      • scripts/test 44 system
      • ✅ 119 specs passed successfully 3:17 Cypress netto, 5:39 script brutto time
    • 👉 Be aware this test only covers the JS usage of pg in Cypress System Tests, Joomla PHP is still using database host jbt_pg with default port number (it would need [3.x] Allow to specify port number or UNIX socket in host option also for MySQL (PDO) and PostgreSQL (PDO) joomla-framework/database#310 - but as it is not focus with this PR, it is ignored)
  4. System Tests to verify change with NPM module pg, IPv6 hostname and non-default port number
    • After step 5 setup continue with changing to use hostname and non-default port number:
      • `db_host: 'jbt_cypress',
      • `db_port: '4711',
    • Running System Tests
  5. System Tests to verify change with NPM module pg, IPv4 and using a hostname and default port number (the default case)
    • scripts/create 44 pgsql https://github.com/muhme/joomla-cms:44-pg-for-postgres
    • Used database host is jbt_pg and db_port is empty
    • Running System Tests (the installation step was already executed during scripts/create)
      • scripts/test 44 system
      • using upto 10 servers sessions, checked with pgAdmin
      • ✅ 119 specs passed successfully 3:22 Cypress netto, 5:47 script brutto time
  6. System Tests to verify change with NPM module pg, using a IPv4 address and non-standard port number
    • With the setup from step # 7 determine IPv4 address:
      • docker exec jbt_cypress ping host.docker.internal -> 192.168.65.254 in my case
    • Configure database and port in cypress.config.mjs file:
      • db_host: '192.168.65.254'
      • db_port: '7003'
    • This setup can be handled by the Joomla Web Installer, allowing us to test the installation step too:
      • scripts/test 44 system install/Installation.cy.js novnc
    • Running System Tests
      • scripts/test 44 system
      • using upto 10 servers sessions, checked with pgAdmin
      • ✅ 119 specs passed successfully 3:07 Cypress netto, 5:27 script brutto time
  7. System Tests to verify change with NPM module pg, using a IPv4 address and standard port number
    • WIth the setup from step # 8 determine IPv4 address:
      • docker exec jbt_cypress ping jbt_pg -> 192.168.150.4 in my case
    • Configure database and port in cypress.config.mjs file:
      • db_host: '192.168.65.4'
      • db_port: ''
    • This setup can be handled by the Joomla Web Installer, allowing us to test the installation step too:
      • scripts/test 44 system install/Installation.cy.js novnc
    • Running System Tests
      • scripts/test 44 system
      • using upto 10 servers sessions, checked with pgAdmin
      • ✅ 119 specs passed successfully 3:08 Cypress netto, 5:28 script brutto time
  8. System Tests to verify change with NPM module pg and using Unix socket
    • For this test case, we also need 44092 to be applied. However, this cannot be applied via Patch Tester, as doing so would cause us to lose the changes related to pg module in this PR. Therefore, the changes from 44092 must be applied manually. You can use the atteched file db.mjs.txt to do this.
    • scripts/create 44 pgsql socket https://github.com/muhme/joomla-cms:44-pg-for-postgres
      • Check: administrator | Global Configuration | Server | Database Host: unix:/jbt/run/postgresql-socket
    • Apply PostgreSQL changes from 44092:
      • cp db.mjs.txt branch_44/tests/System/plugins/db.mjs
    • Running System Tests
      • scripts/test 44 system
      • using upto 10 servers sessions, checked with pgAdmin
      • ✅ 119 specs passed successfully 3:45 Cypress netto, 6:08 script brutto time
      • Surprisingly, Unix sockets are slower than TCP/IP communication, which could be due to the Unix sockets being volume-mounted in the Docker setup.

Actual result BEFORE applying this Pull Request

  • The custom database commands in the System Tests are failing for PostgreSQL when using IPv6 addresses.

Expected result AFTER applying this Pull Request

  • The System Tests are passing for PostgreSQL with IPv6 addresses, including those with non-standard port numbers, host names, IPv4 addresses, and Unix sockets.

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

@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 Sep 16, 2024
@laoneo
Copy link
Member

laoneo commented Sep 16, 2024

At the time I implemented Postgres into the system tests, there was a reason why I didn't use pg, but can't remember anymore.

@muhme
Copy link
Contributor Author

muhme commented Sep 16, 2024

At the time I implemented Postgres into the system tests, there was a reason why I didn't use pg, but can't remember anymore.

The dependencies? Or the MIT license?

For functionality I will test - beside IPv6 - number of sessions, run time, Unix sockets ...

@laoneo
Copy link
Member

laoneo commented Sep 16, 2024

It was something with the code.

@richard67
Copy link
Member

Drone reports JavaScript code style issues: https://ci.joomla.org/joomla/joomla-cms/78949/1/33

@muhme muhme changed the title Using 'pg' for 'postgres' [cypress] Using NPM Module 'pg' for 'postgres' Sep 17, 2024
@muhme muhme marked this pull request as ready for review September 24, 2024 13:11
@alikon
Copy link
Contributor

alikon commented Sep 28, 2024

I have tested this item ✅ successfully on 7befd05


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

@laoneo
Copy link
Member

laoneo commented Oct 20, 2024

Can you rebase this one to 5.2?

For Joomla 4.4-dev the same as
2e8dc3f

Additionally updated to current 'pg' version 8.13.0
@muhme muhme marked this pull request as draft October 22, 2024 13:00
@joomla-cms-bot joomla-cms-bot removed the NPM Resource Changed This Pull Request can't be tested by Patchtester label Oct 22, 2024
@muhme muhme changed the base branch from 4.4-dev to 5.2-dev October 22, 2024 13:01
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev labels Oct 22, 2024
@muhme
Copy link
Contributor Author

muhme commented Oct 22, 2024

After rebase on 5.2-dev plus update to current pg version 8.13.0 tested with JBT in creating instance from PR and running System Tests:

scripts/create 52 IPv6 pgsql  https://github.com/muhme/joomla-cms:44-pg-for-postgres 
scripts/test 52 system

All tests are passed ✅

@muhme muhme marked this pull request as ready for review October 22, 2024 13:50
@muhme muhme removed the PR-4.4-dev label Oct 22, 2024
@Hackwar Hackwar merged commit 401afed into joomla:5.2-dev Oct 23, 2024
3 checks passed
@Hackwar Hackwar added this to the Joomla! 5.2.1 milestone Oct 23, 2024
@Hackwar
Copy link
Member

Hackwar commented Oct 23, 2024

Thanks for this fix.

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-5.2-dev Unit/System Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants