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

rewrite tests #515

Merged
merged 3 commits into from
Apr 16, 2022
Merged

rewrite tests #515

merged 3 commits into from
Apr 16, 2022

Conversation

williamh
Copy link
Contributor

  • rewrite tests to work with meson
  • add tests to ci

@williamh
Copy link
Contributor Author

This will fix #513 once it is merged.

@williamh
Copy link
Contributor Author

I need some folks from Debian/Ubuntu/Alpine to take a look and tell me what I'm missing.
The tests work on Gentoo, even when I rewrite the code to use all pre 0.56 meson code.
However, as you can see on this pr, the tests break CI for your distros.
Please let me know what I should do.

@eli-schwartz
Copy link

Not a user of any of those, however I may have some suggestions.

It's a weird error message, but that being said -- this script uses /bin/bash instead of /bin/sh and Alpine prefers not to have bash in the base setup, while the BSDs typically put bash in /usr/local/bin due to hysterical raisins.

@williamh
Copy link
Contributor Author

Hi @eli-schwartz , That was a typo actually, all of the test scripts are meant to be sh scripts. I made that change, and Alpine is the only one that is failing now. Can you take a look?

Thanks much,

William

@TheOneric
Copy link

TheOneric commented Apr 13, 2022

It appears this test already failed at 0efc1b1 in Alpine.
Looking at the POSIX sed specification the issue appears to be that q does not take an exit code argument in POSIX, but can in GNU sed. Removing the 1 gets rid of the command parsing error, but of course also means it no longer errors on trailing empty lines. Curiously FreeBSD’s sed does not document an argument for q either in its man page. Is the CI using GNU sed from ports instead of the system’s default sed?
If there’s no reason to avoid awk, the following should do the trick while using only POSIX features iinm:

--- a/test/check-trailing-newlines.sh
+++ b/test/check-trailing-newlines.sh
@@ -6,7 +6,7 @@ top_srcdir=${SOURCE_ROOT:-..}
 ebegin "Checking trailing newlines in code"
 out=$(cd ${top_srcdir};
        for f in `find */ -name '*.[ch]'` ; do
-               sed -n -e :a -e '/^\n*$/{$q1;N;ba' -e '}' $f || echo $f
+               awk '/^$/ {ret=1; next} 1 {ret=0} END {exit ret}' $f || echo $f
        done)
 [ -z "${out}" ]
 eend $? "Trailing newlines need to be deleted:"$'\n'"${out}"

(EDIT:) Or alternatively using tail might be faster: tail -n 1 $f | grep -Eq '^$' && echo $f

This ports our tests to meson and makes them able to be run in parallel.
This test was using a GNU sed command which does not work on Alpine Linux.
@williamh williamh force-pushed the rewrite-tests branch 3 times, most recently from 99e9177 to 1ae6ec8 Compare April 16, 2022 20:03
@williamh williamh merged commit fdfa6db into OpenRC:master Apr 16, 2022
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.

4 participants