GNOME Bugzilla – Bug 737194
ostree admin: Fix return value from 'ostree admin'
Last modified: 2014-09-25 18:30:27 UTC
'ostree admin' with no arguments was meant to fail, but the logic was wrong; add an assertion on the return value from all ostree commands to catch similar problems in the future.
Created attachment 286891 [details] [review] ostree admin: Fix return value from 'ostree admin'
Review of attachment 286891 [details] [review]: Could add a simple test case for this too; look at what test-basic.sh does to scrape for an error message.
Created attachment 286983 [details] [review] Pass --help to the most nested subcommand 'ostree admin <x> --help' and 'ostree admin instutil <x> --help' should give help on the deepest subcommand, not on 'ostree admin'.
Created attachment 286984 [details] [review] Fix help output for nested subcommands Only 'ostree admin' was appearing in the help message for commands nested within ostree admin.
Created attachment 286985 [details] [review] ostree admin instutil: make --help work for subcommands Even though the subcommands don't take arguments, use a GOptionContext so that --help works as expected.
I've attached some more related fixes for --help. I'm a bit confused about the test framework, when I do: $ jhbuild run gnome-desktop-testing-runner ostree I get a pile of failures: SUMMARY: total=27; passed=10; skipped=3; failed=14; user=4.3s; system=1.5s; maxrss=34108 Is there some special way to run it. What are you looking for for a test - just that 'ostree admin' exists with a non-zero exit status and 'ostree admin --help' exits with a zero exit status?
(In reply to comment #6) > I've attached some more related fixes for --help. I'm a bit confused about the > test framework, when I do: > > $ jhbuild run gnome-desktop-testing-runner ostree > > I get a pile of failures: > > SUMMARY: total=27; passed=10; skipped=3; failed=14; user=4.3s; system=1.5s; > maxrss=34108 What are the error messages? Are you hitting: https://bugzilla.gnome.org/show_bug.cgi?id=732184 ? > Is there some special way to run it. What are you looking for for a test - just > that 'ostree admin' exists with a non-zero exit status and 'ostree admin > --help' exits with a zero exit status? Something like that, yeah.
Created attachment 287011 [details] Log of errors from trying to run test case Here's the log of my testing run. Some selected errors: error: Union checkout of e1fa95de2b8338fd7b4e6aea46f65c89151fadecc59ad883b423c57531639bbe to somelink: lsetxattr: No such file or directory error: Error creating directory: File exists error: Error making symbolic link: File exists error: corrupted object d5956bc83ed942a15471beeeb83276a393750ae86126bcc91448abe2098b9c73.file; actual checksum: a5a3f588be584a08f657a3c02c37982b0847c705d30132db38c1bc8ab7c8b3ea error: Repository corruption encountered
Review of attachment 286983 [details] [review]: This is a good improvement, but it makes the "want_help" boolean dead, right? Basically we just ignore --help?
Review of attachment 286984 [details] [review]: Looks right.
Review of attachment 286985 [details] [review]: Right.
(In reply to comment #8) > Created an attachment (id=287011) [details] > Log of errors from trying to run test case > > Here's the log of my testing run. Some selected errors: > > error: Union checkout of > e1fa95de2b8338fd7b4e6aea46f65c89151fadecc59ad883b423c57531639bbe to somelink: > lsetxattr: No such file or directory Hmm...would need to narrow this down to a specific test case. It's a little annoying to do with g-d-t-r in console mode. If the tests are logging to the journal, they'll include the test id. For the others though, we are hitting the deprecation error...I'll try again to fix it.
(In reply to comment #9) > Review of attachment 286983 [details] [review]: > > This is a good improvement, but it makes the "want_help" boolean dead, right? > Basically we just ignore --help? Not quite, with the previous patch: ostree admin prints usage, followed by 'No command specified' and exits with a non-zero error message ostree admin --help prints usage and exits successfully
(In reply to comment #12) > (In reply to comment #8) > > Created an attachment (id=287011) [details] [details] > > Log of errors from trying to run test case > > > > Here's the log of my testing run. Some selected errors: > > > > error: Union checkout of > > e1fa95de2b8338fd7b4e6aea46f65c89151fadecc59ad883b423c57531639bbe to somelink: > > lsetxattr: No such file or directory > > Hmm...would need to narrow this down to a specific test case. It's a little > annoying to do with g-d-t-r in console mode. If the tests are logging to the > journal, they'll include the test id. > > For the others though, we are hitting the deprecation error...I'll try again to > fix it. With G_ENABLE_DIAGNOSTIC=0 jhbuild run gnome-desktop-testing-runner --report-directory=/tmp/foo ostree, the two failing tests are test-basic which is dying with: error: Union checkout of e1fa95de2b8338fd7b4e6aea46f65c89151fadecc59ad883b423c57531639bbe to somelink: lsetxattr: No such file or directory and test-admin-deploy-switch which is dying with: grep: sysroot/ostree/deploy/testos/current/usr/include/foo.h: No such file or directory File 'sysroot/ostree/deploy/testos/current/usr/include/foo.h' doesn't match regexp 'header' IF you can't reproduce, let me know and I can investigate further. (Having drifted off the topic of this bug.)
Created attachment 287043 [details] [review] Add test for the behavior of --help Recursive over ostree and all subcommands, and check that --help is supported, properly outputs to standard out, and exits with a 0 exit status. Check that for commands with subcommands, they produce the help output to standard error when run with no arguments.
Created attachment 287044 [details] [review] --help should always go to stdout The standard convention is that the output of --help should go to standard output (so that it can be piped to a pager and searched.) See, e.g., the GNU coding standards.
Created attachment 287045 [details] [review] ostree admin: Fix return value from 'ostree admin [instutil]' New version of the initial patch that also fixes up 'ostree admin instutil' and that prevents 'ostree' with no arguments from triggering the assertion.
Review of attachment 287043 [details] [review]: One minor comment, otherwise looks good, and please commit after addressing. ::: tests/libtest.sh @@ +80,3 @@ +assert_file_empty() { + if grep -q . "$1"; then test -s ?
Review of attachment 287044 [details] [review]: Right.
Review of attachment 287045 [details] [review]: Makes sense.
Attachment 286983 [details] pushed as 1dca556 - Pass --help to the most nested subcommand Attachment 286984 [details] pushed as 9d72ff2 - Fix help output for nested subcommands Attachment 286985 [details] pushed as c9018c7 - ostree admin instutil: make --help work for subcommands Attachment 287043 [details] pushed as 7fce7e0 - Add test for the behavior of --help Attachment 287044 [details] pushed as 3400f2d - --help should always go to stdout Attachment 287045 [details] pushed as 40f490e - ostree admin: Fix return value from 'ostree admin [instutil]'