On Monday 14 March 2016, Stanislav Brabec wrote: > On Mar 14, 2016 at 02:20 Ruediger Meier wrote: > > On Tuesday 02 February 2016, Stanislav Brabec wrote: > > IMO this test needs some more error handling, see below. > > > >> +btrfs subvol create s2 >/dev/null > > > > For example if you comment out above line (or if it would fail) > > then all subtests below still succeed. This can't be right. > > Well, it is a test suite for mount, not btrfs. I did not add any > error checks in the code that creates the testing image. > Grepping through the whole test suite, I see that more than a half of > all tests do not check for mkfs failure. Here we just have a > multi-line mkfs. > > It is possible to add a check for every command there, and fail the > test if the command fails or returns something unexpected. You are right. I've added already some lines to skip outdated btrfs-tools. Nethertheless both subtests succeed even when they operate on an empty device without any filesystem or subvolume. Ideally a test should be skipped if the system is broken. But if we don't have "skip rules" implemented yet then it should better fail than succeed. I know that we have a lot of such issues in our test suite. Just commented this one because it was recently added and I've noticed it on existing systems. > >> +NON_DEFAULT_SUBVOLID=$(btrfs subvol list "$TS_MOUNTPOINT-create" > >> | > > NON_DEFAULT_SUBVOLID will be empty. > > >> "subvolid=$NON_DEFAULT_SUBVOLID" + > > I am not sure how kernel will interpret subvolid=. Maybe just log the btrfs return codes to TS_OUTPUT. If we see failure in real life then we know that something has to be fixed, either the skip rules or the code to create subvols. cu, Rudi -- To unsubscribe from this list: send the line "unsubscribe util-linux" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
