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

Use PropertyResourceBundle in test suite and validate .properties files are not both valid UTF-8 and valid ISO-8859-1 #433

Merged
merged 3 commits into from
May 10, 2022
Merged

Use PropertyResourceBundle in test suite and validate .properties files are not both valid UTF-8 and valid ISO-8859-1 #433

merged 3 commits into from
May 10, 2022

Conversation

timja
Copy link
Member

@timja timja commented May 10, 2022 •
edited
Loading

see jenkinsci/jenkins#6550 (comment) jenkinsci/stapler#357

You can't actually use a file that is valid utf-8 and not valid ISO-8859-1 in at least core right now due to https://github.com/jenkinsci/jenkins/blob/ff39fbb3cdf58d7dda17c37b8d0606dc8e9282f4/core/pom.xml#L604

But we can adjust that later when we switch to Java 11

Validated with core:

[ERROR] Tests run: 7139, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.743 s <<< FAILURE! - in jenkins.CorePropertiesTest
[ERROR] hudson/Messages_es.properties(org.jvnet.hudson.test.PropertiesTestSuite$PropertiesTest)  Time elapsed: 0.004 s  <<< FAILURE!
java.lang.AssertionError: jar:file:/Users/timja/.m2/repository/org/jenkins-ci/main/jenkins-core/2.347-SNAPSHOT/jenkins-core-2.347-SNAPSHOT.jar!/hudson/Messages_es.properties is encoded with both valid utf-8 and ISO-8859-1 characters, the utf-8 characters need to be encoded with something like `native2ascii`
	at org.jvnet.hudson.test.PropertiesTestSuite$PropertiesTest.runTest(PropertiesTestSuite.java:87)

@timja timja mentioned this pull request May 10, 2022
Merged
11 tasks
@timja timja requested review from jglick and basil May 10, 2022 06:35
basil
basil approved these changes May 10, 2022
View reviewed changes
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

src/main/java/org/jvnet/hudson/test/PropertiesTestSuite.java Outdated Show resolved Hide resolved
src/main/java/org/jvnet/hudson/test/PropertiesTestSuite.java Outdated Show resolved Hide resolved
@basil basil changed the title Use PropertyResourceBundle in test suite, validate characters are not valid in both utf-8 and ISO-8859-1 Use PropertyResourceBundle in test suite and validate .properties files are not both valid UTF-8 and valid ISO-8859-1 May 10, 2022
jglick
jglick approved these changes May 10, 2022
View reviewed changes
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Other than the above nits, looks nice! Thanks for taking care of this.

src/main/java/org/jvnet/hudson/test/PropertiesTestSuite.java Outdated Show resolved Hide resolved
@timja timja enabled auto-merge (squash) May 10, 2022 18:11
@timja timja merged commit 86627ac into jenkinsci:master May 10, 2022
@timja timja deleted the update-properties-loading branch May 10, 2022 18:59
@basil basil mentioned this pull request Jun 30, 2022
Merged
@ikedam ikedam mentioned this pull request Jan 14, 2023
Merged
6 tasks
@basil basil mentioned this pull request Oct 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@basil basil basil approved these changes

@jglick jglick jglick approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@timja @basil @jglick