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.

Sign up for GitHub

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

Jump to bottom

Fixes issue #1173 with translator.t + Downgrade Travis worker to make Mysql work #1175

Merged
merged 6 commits into from
Jun 28, 2024
Merged

Fixes issue #1173 with translator.t + Downgrade Travis worker to make Mysql work #1175

merged 6 commits into from
Jun 28, 2024

Conversation

matsduf
Copy link
Contributor

@matsduf matsduf commented Jun 25, 2024 •
edited by MichaelTimbert
Loading

Purpose

Purpose 1

This PR makes locale being set so that translation works in t/translator.t in all OSs.

Travis is an exception where it has not been possible to make it work for all subtests. To prevent it from failing the script ends before all subtests have been run.

Purpose 2

The MySQL service suddenly fails to start in Jammy, but it still works in Focal. For Postgresql Jammy must be kept.

Context

Fixes #1173 and fixes issue with failing Mysql in Travis

How to test this PR

Purpose 1

  • Travis must pass the first few subtests from translator.t.
  • Every supported OSs must pass the tests when the calling environment is without locale with UTF-8 support.
    • Run printenv to identify all set locale related keys (value being C.UTF-8, C or some other value).
    • Remove all keys that have a value containing the string "utf" or "UTF".
    • Run translator.t and all tests should pass.
    • Select OS when tested and has passed.
  • Debian 12
  • FreeBSD 14.1
  • Rocky Linux 8.10
  • Rocky Linux 9.4
  • Ubuntu 20.04
  • Ubuntu 22.04
  • Ubuntu 24.04

Purpose 2

Travis should pass.

@matsduf
Copy link
Contributor Author

matsduf commented Jun 25, 2024

Well, on Debian 12 it seems to work.

@matsduf
Copy link
Contributor Author

matsduf commented Jun 25, 2024

There is another unrelated error from Travis:

$ if [[ "$TARGET" == "MySQL" ]]; then mysql -u root -e "CREATE USER 'travis_zm'@'localhost' IDENTIFIED BY '*****************';"; fi

ERROR 2002 (HY000): Can't connect to local MySQL server through socket '/var/run/mysqld/mysqld.sock' (2)

@matsduf matsduf mentioned this pull request Jun 25, 2024
Open
marc-vanderwal
marc-vanderwal previously approved these changes Jun 26, 2024
View reviewed changes
Copy link
Contributor

@marc-vanderwal marc-vanderwal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad you found the root cause. Those pesky environment variables!

mattias-p
mattias-p reviewed Jun 26, 2024
View reviewed changes
t/translator.t Outdated Show resolved Hide resolved
@matsduf
Copy link
Contributor Author

matsduf commented Jun 26, 2024

Glad you found the root cause. Those pesky environment variables!

Unfortunetely, there is more to it.

@tgreenx
Copy link
Contributor

tgreenx commented Jun 26, 2024

Glad you found the root cause. Those pesky environment variables!

Unfortunetely, there is more to it.

Yes, for sure. For example on OSes where only one test fail, i.e a Backend specific message, where the locale is correctly set.

@tgreenx tgreenx added this to the v2024.1 milestone Jun 26, 2024
@mattias-p
Copy link
Member

Regarding the MySQL problem I noticed this: https://app.travis-ci.com/github/zonemaster/zonemaster-backend/jobs/623371555#L1060

$ sudo systemctl start mysql
Failed to start mysql.service: Unit mysql.service not found.
MySQL did not start within 10 seconds

@tgreenx tgreenx linked an issue Jun 26, 2024 that may be closed by this pull request
Unit test translator.t fails on Debian #1173
Open
@matsduf matsduf force-pushed the fix-issue-translation.t branch from e0fe0f8 to a19f98c Compare June 26, 2024 14:40
@matsduf
Updates t/translator.t
2f27b7b 
* Sets correct environment to make it pass under all OSs.
* Adds a breakout when run under Travis to work-around an issue
  under Travis not passing the tests.
@matsduf matsduf force-pushed the fix-issue-translation.t branch from a19f98c to 2f27b7b Compare June 26, 2024 14:52
@matsduf
Copy link
Contributor Author

matsduf commented Jun 26, 2024

Regarding the MySQL problem I noticed this: https://app.travis-ci.com/github/zonemaster/zonemaster-backend/jobs/623371555#L1060

$ sudo systemctl start mysql
Failed to start mysql.service: Unit mysql.service not found.
MySQL did not start within 10 seconds

It should be fixed in a separate PR.

@matsduf
Copy link
Contributor Author

matsduf commented Jun 26, 2024

@marc-vanderwal and @mattias-p, please re-review. I have used the model proposed by @mattias-p on our meeting. Disregard the Mysql issue for this PR.

mattias-p
mattias-p previously approved these changes Jun 26, 2024
View reviewed changes
@mattias-p
Downgrade Travis worker
8d86d10 
The MySQL service suddenly fails to start in Jammy, but it still works
in Focal.
@mattias-p
Copy link
Member

I've created matsduf#7 to update this PR with a fix for the MySQL problem.

@matsduf matsduf changed the title Fixes issue #1173 with translation.t Fixes issue #1173 with translation.t + Downgrade Travis worker to make Mysql work Jun 27, 2024
marc-vanderwal
marc-vanderwal previously approved these changes Jun 27, 2024
View reviewed changes
@mattias-p
Copy link
Member

It seem Jammy can't handle MySQL and Focal can't handle PostgreSQL. But it's possible to specify dist per job like this: ab41ad3.

@matsduf
Copy link
Contributor Author

matsduf commented Jun 27, 2024

It seem Jammy can't handle MySQL and Focal can't handle PostgreSQL. But it's possible to specify dist per job like this: ab41ad3.

Already solved that in the same way, but with a different setting.

@matsduf
Copy link
Contributor Author

matsduf commented Jun 27, 2024

@tgreenx @mattias-p @marc-vanderwal @MichaelTimbert: Please review, and then verify that "your" OS works correctly. See "how to test".

mattias-p
mattias-p previously approved these changes Jun 27, 2024
View reviewed changes
tgreenx
tgreenx approved these changes Jun 27, 2024
View reviewed changes
Copy link
Contributor

@tgreenx tgreenx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debian 12:

$ printenv | grep -i utf
$ git log -1 --oneline
81d55a4 (HEAD -> test-PR1175) Keeps Jammy for Postgresql
$ prove t/translator.t 
t/translator.t .. ok   
All tests successful.
Files=1, Tests=9,  2 wallclock secs ( 0.05 usr  0.00 sys +  1.69 cusr  0.08 csys =  1.82 CPU)
Result: PASS

Ubuntu 24.04:

$ printenv | grep -i utf
$ git log -1 --oneline
81d55a4 (HEAD -> test-PR1175) Keeps Jammy for Postgresql
zonemaster@zm-2024:~/zonemaster-backend$ printenv | grep -i utf
$ prove t/translator.t 
t/translator.t .. ok   
All tests successful.
Files=1, Tests=9,  3 wallclock secs ( 0.07 usr  0.02 sys +  2.78 cusr  0.43 csys =  3.30 CPU)
Result: PASS

@marc-vanderwal
Copy link
Contributor

Testing completed successfully on Rocky 9:

$ git log -1 --oneline
81d55a45 (HEAD -> fix-issue-translation.t, matsduf/fix-issue-translation.t) Keeps Jammy for Postgresql
$ printenv | grep -i utf
LANG=en_US.UTF-8
$ unset LANG
$ printenv | grep -i utf
$ prove t/translator.t
t/translator.t .. ok
All tests successful.
Files=1, Tests=9,  2 wallclock secs ( 0.02 usr  0.00 sys +  0.69 cusr  0.09 csys =  0.80 CPU)
Result: PASS

marc-vanderwal
marc-vanderwal previously approved these changes Jun 27, 2024
View reviewed changes
@tgreenx tgreenx changed the title Fixes issue #1173 with translation.t + Downgrade Travis worker to make Mysql work Fixes issue #1173 with translator.t + Downgrade Travis worker to make Mysql work Jun 27, 2024
tgreenx
tgreenx requested changes Jun 27, 2024
View reviewed changes
t/translator.t Outdated Show resolved Hide resolved
@matsduf matsduf dismissed stale reviews from marc-vanderwal and mattias-p via d320957 June 27, 2024 14:23
@matsduf @tgreenx
Update t/translator.t
d320957 
Co-authored-by: tgreenx <[email protected]>
@MichaelTimbert
Copy link
Contributor

fix work on Ubuntu 20.04.

$ prove t/translator.t
t/translator.t .. ok
All tests successful.
Files=1, Tests=9,  1 wallclock secs ( 0.02 usr  0.00 sys +  0.72 cusr  0.05 csys =  0.79 CPU)
Result: PASS

@MichaelTimbert
Copy link
Contributor

fix work on Ubuntu 22.04.

$ prove t/translator.t
t/translator.t .. ok
All tests successful.
Files=1, Tests=9,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.29 cusr  0.02 csys =  0.32 CPU)
Result: PASS

@matsduf matsduf merged commit 8f85f98 into zonemaster:develop Jun 28, 2024
1 check passed
@matsduf matsduf deleted the fix-issue-translation.t branch June 28, 2024 07:52
@matsduf matsduf added T-Bug Type: Bug in software or error in test case description V-Patch Versioning: The change gives an update of patch in version. labels Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattias-p mattias-p mattias-p approved these changes

@tgreenx tgreenx tgreenx approved these changes

@MichaelTimbert MichaelTimbert Awaiting requested review from MichaelTimbert

@marc-vanderwal marc-vanderwal Awaiting requested review from marc-vanderwal

Assignees
No one assigned
Labels
T-Bug Type: Bug in software or error in test case description V-Patch Versioning: The change gives an update of patch in version.
Projects
None yet
Milestone
v2024.1
Development

Successfully merging this pull request may close these issues.

Unit test translator.t fails on Debian
5 participants
@matsduf @tgreenx @mattias-p @marc-vanderwal @MichaelTimbert