Hi，
在 2015年06月12日 20:22, Pino Toscano 写道:
Hi,
On Friday 12 June 2015 19:54:26 Cao Jin wrote:
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
While I appreciate the effort you are putting in writing and testing
the new API, and the explanation about the added calls in checks,
in my opinion the added tests are not (in they way they are added)
useful.
The problem is that if I, for example, make the btrfs_replace_start
implementation a no-op (for example by having just "return 0" in it),
the test would keep working as before, as the status-quo before and
after each block of btrfs-replace-start calls is the same, and nobody
would notice if not by running it.
Sure, making it a no-op is a stretch, sure, but image if
`btrfs replace` would suddenly do nothing: the result would be the same
as above. Currently, the only thing checked is that the
btrfs_replace_start calls don't fail, but they tell me nothing about
whether each of them did the right thing.
I am aware of your concern, based on the unsmooth experience of the
command. The existed btrfs_device_add/delete tests in
test-btrfs-devices.sh may also have the same problem(like no-op but just
return 0, or suddenly do nothing)
That's why I would rather prefer a separate test for
btrfs_replace_start, either a new guestfish calls in
test-btrfs-devices.sh (using the disks created there already) or as a
separate new bash/perl test altogether.
Ok, I will put the tests in a separate new bash, try to do the tests
more accurately.
--
Yours Sincerely,
Cao Jin
_______________________________________________
Libguestfs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libguestfs
