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

Do not test YCP on non-UTF-8 strings (bsc#1217523) #165

Merged
merged 6 commits into from
Nov 28, 2023

Conversation

shundhammer
Copy link
Contributor

@shundhammer shundhammer commented Nov 27, 2023

Bugzilla

https://bugzilla.suse.com/show_bug.cgi?id=1217523

Problem

Build failure with a newer rpm package: The libycp testsuite fails.

Cause

Non-UTF-8-encoded strings cause grep to detect a binary file and report "binary file matches", not the expected output.

Fix

  • Removed the one individual test that made grep detect the stderr file for test builtin/Builtin-String1 as a binary file.

  • Removed the entire values/SingleCharStrings test. That one was useful while developing the YCP parser, but now it only causes trouble because it contains a ton of special characters (char 0 .. char 31, Ctrl-A .. Ctrl-Z; a ton of backslash character literals >char 127). That causes grep to detect the source file, its stdout and its stderr file as binary files. Nothing good can ever come from that.

  • Added usable documentation how to run the tests and fix them if there are problems. That stuff cost me a whole working day.

    Preformatted version: https://github.com/yast/yast-core/blob/master/libycp/testsuite/README.md

Test

Built and ran the test on the latest TW.

@shundhammer shundhammer force-pushed the huha-fix-testsuite branch 2 times, most recently from 019d967 to 7ff171d Compare November 27, 2023 17:11
@shundhammer shundhammer marked this pull request as ready for review November 28, 2023 13:13
Copy link
Member

@mvidner mvidner left a comment

Choose a reason for hiding this comment

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

Thanks, also for the README

With a more informative changes entry, I think the PR title should be updated too :)

package/yast2-core.changes Outdated Show resolved Hide resolved
Co-authored-by: Martin Vidner <mvidner@suse.cz>
@shundhammer shundhammer changed the title Recoded ISO-8859-1 to UTF-8 to prevent build failure (bsc#1217523) Do not test YCP on non-UTF-8 strings (bsc#1217523) Nov 28, 2023
@shundhammer shundhammer merged commit f024117 into master Nov 28, 2023
2 checks passed
@shundhammer shundhammer deleted the huha-fix-testsuite branch November 28, 2023 14:34
@yast-bot
Copy link
Contributor

✔️ Internal Jenkins job #26 successfully finished
✔️ Created OBS submit request #1129630

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants