Re: [Libguestfs] [PATCH] New API: btrfs_replace_start

>>>> diff --git a/tests/btrfs/test-btrfs-devices.sh b/tests/btrfs/test-btrfs-devices.sh >>>> index 3935c60..463b0a8 100755 >>>> --- a/tests/btrfs/test-btrfs-devices.sh >>>> +++ b/tests/btrfs/test-btrfs-devices.sh >>>> @@ -66,6 +66,8 @@ btrfs-device-add "/dev/sdc1 /dev/sdd1" / >>>> btrfs-device-delete "/dev/sdb1" / >>>> btrfs-device-add "/dev/sdb1" / >>>> btrfs-device-delete "/dev/sdc1 /dev/sdd1" / >>>> +btrfs-replace-start "/dev/sda1" "/dev/sdd1" / force:true >>>> +btrfs-replace-start "/dev/sdd1" "/dev/sda1" / force:true >>>> >>>> mkdir /data2 >>>> tar-in $srcdir/../data/filesanddirs-10M.tar.xz /data2 compress:xz >>>> @@ -74,6 +76,8 @@ btrfs-device-add "/dev/sdc1 /dev/sdd1" / >>>> btrfs-device-delete "/dev/sdb1" / >>>> btrfs-device-add "/dev/sdb1" / >>>> btrfs-device-delete "/dev/sdc1 /dev/sdd1" / >>>> +btrfs-replace-start "/dev/sda1" "/dev/sdd1" / force:true >>>> +btrfs-replace-start "/dev/sdd1" "/dev/sda1" / force:true >>>> >>>> mkdir /data3 >>>> tar-in $srcdir/../data/filesanddirs-10M.tar.xz /data3 compress:xz >>>> @@ -82,6 +86,8 @@ btrfs-device-add "/dev/sdc1 /dev/sdd1" / >>>> btrfs-device-delete "/dev/sdb1" / >>>> btrfs-device-add "/dev/sdb1" / >>>> btrfs-device-delete "/dev/sdc1 /dev/sdd1" / >>>> +btrfs-replace-start "/dev/sda1" "/dev/sdd1" / force:true >>>> +btrfs-replace-start "/dev/sdd1" "/dev/sda1" / force:true >>>> >>>> mkdir /data4 >>>> tar-in $srcdir/../data/filesanddirs-10M.tar.xz /data4 compress:xz >>>> @@ -90,6 +96,8 @@ btrfs-device-add "/dev/sdc1 /dev/sdd1" / >>>> btrfs-device-delete "/dev/sdb1" / >>>> btrfs-device-add "/dev/sdb1" / >>>> btrfs-device-delete "/dev/sdc1 /dev/sdd1" / >>>> +btrfs-replace-start "/dev/sda1" "/dev/sdd1" / force:true >>>> +btrfs-replace-start "/dev/sdd1" "/dev/sda1" / force:true >>> >>> What are these tests supposed to check? Other than calling >>> btrfs-replace-start and checking it does not fail, how can the result >>> of this operation be actually checked? >>> >> >> These tests are used for test whether btrfs replace will success or not. >> The existed add/delete test cases may also have the "problem" you >> mentioned: don`t know how to actually check the result, like, is the >> device really added/deleted in the btrfs? I have tested the api both in >> guestfish and the test case script, in guestfish, it is easy to check >> whether the device is replaced or not, just mount and check(of course, >> the api worked). In test case, it is not convenient. But actually, in >> test case, if btrfs-replace-start fails, the script will exit with >> errors, I encountered this situation when debug this case. >> Just "replace sda with sdd" is not enough for the test case, but after >> adding "replace sdd with sda", I think it is pretty sure that the case >> can actually check the result. Because even if 1st replace exit without >> error but actually not replaced, the 2nd replace will exit with error. >> So the 2nd replace add assurance. Also, I have ran the case successfully. > > This is nice description, but still we need *automated* test cases, > otherwise checking that the API works is a nightmare. > Yes, the test cases should be automated. Maybe there is a litter mistake here. what I want say is: the tests I wrote, is automated, and the result is capable of checking the result actually. I mentioned a little about how it can actually check the result. As you know, the test script under tests/btrfs/ are automated, my tests are added in the test-btrfs-devices.sh, so, my api test is automated. And the rest we should concern is that, could the tests check the result actually? Answer is: Yes. Here is the thing: the tests I wrote: A: btrfs-replace-start "/dev/sda1" "/dev/sdd1" / force:true B: btrfs-replace-start "/dev/sdd1" "/dev/sda1" / force:true When I use the api without option "-B"(the api won`t work with "-B"), the tests can`t pass, will fail at B, because A actally didn`t work(but don`t fail at A), so B will fail. I add B, just for the purpose that it can actually check the api`s result. I mentioned I have tested the api in guestfish, means that I am sure this api can work