xfstests: more tests for test case btrfs/030
diff mbox

Message ID 1391220332-22118-1-git-send-email-fdmanana@gmail.com
State Not Applicable
Headers show

Commit Message

Filipe Manana Feb. 1, 2014, 2:05 a.m. UTC
This change adds some new tests for btrfs' incremental send feature.
These are all related with inverting the parent-child relationship
of directories, and cover the cases:

* when the new parent didn't get renamed (just moved)
* when a child file of the former parent gets renamed too

These new cases are fixed by the following btrfs linux kernel patches:

* "Btrfs: more send support for parent/child dir relationship inversion"
* "Btrfs: fix send dealing with file renames and directory moves"

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 tests/btrfs/030 |   86 +++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 65 insertions(+), 21 deletions(-)

Comments

Dave Chinner Feb. 2, 2014, 9:57 p.m. UTC | #1
On Sat, Feb 01, 2014 at 02:05:32AM +0000, Filipe David Borba Manana wrote:
> This change adds some new tests for btrfs' incremental send feature.
> These are all related with inverting the parent-child relationship
> of directories, and cover the cases:
> 
> * when the new parent didn't get renamed (just moved)
> * when a child file of the former parent gets renamed too
> 
> These new cases are fixed by the following btrfs linux kernel patches:
> 
> * "Btrfs: more send support for parent/child dir relationship inversion"
> * "Btrfs: fix send dealing with file renames and directory moves"
> 
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>

Rather than modifying 030 which will cause it to fail on kernels
where it previously passed, can you factor out the common code and
create a new test with the additional coverage?

i.e. the rule of thumb is that once a test is "done" we don't go
back and modify it in significant ways - we write a new unit test
that covers the new/extended functionality. Redundancy in unit tests
is not a bad thing...

Cheers,

Dave.
Filipe Manana Feb. 2, 2014, 10:08 p.m. UTC | #2
On Sun, Feb 2, 2014 at 9:57 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Sat, Feb 01, 2014 at 02:05:32AM +0000, Filipe David Borba Manana wrote:
>> This change adds some new tests for btrfs' incremental send feature.
>> These are all related with inverting the parent-child relationship
>> of directories, and cover the cases:
>>
>> * when the new parent didn't get renamed (just moved)
>> * when a child file of the former parent gets renamed too
>>
>> These new cases are fixed by the following btrfs linux kernel patches:
>>
>> * "Btrfs: more send support for parent/child dir relationship inversion"
>> * "Btrfs: fix send dealing with file renames and directory moves"
>>
>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>
> Rather than modifying 030 which will cause it to fail on kernels
> where it previously passed, can you factor out the common code and
> create a new test with the additional coverage?
>
> i.e. the rule of thumb is that once a test is "done" we don't go
> back and modify it in significant ways - we write a new unit test
> that covers the new/extended functionality. Redundancy in unit tests
> is not a bad thing...

Right. The only reason I did this, instead of a new test file, is that
because the former fix which btrfs/030 relates to is not yet in any
kernel release. Given this fact, what do you think?

thanks

>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
Dave Chinner Feb. 2, 2014, 10:25 p.m. UTC | #3
On Sun, Feb 02, 2014 at 10:08:06PM +0000, Filipe David Manana wrote:
> On Sun, Feb 2, 2014 at 9:57 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Sat, Feb 01, 2014 at 02:05:32AM +0000, Filipe David Borba Manana wrote:
> >> This change adds some new tests for btrfs' incremental send feature.
> >> These are all related with inverting the parent-child relationship
> >> of directories, and cover the cases:
> >>
> >> * when the new parent didn't get renamed (just moved)
> >> * when a child file of the former parent gets renamed too
> >>
> >> These new cases are fixed by the following btrfs linux kernel patches:
> >>
> >> * "Btrfs: more send support for parent/child dir relationship inversion"
> >> * "Btrfs: fix send dealing with file renames and directory moves"
> >>
> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> >
> > Rather than modifying 030 which will cause it to fail on kernels
> > where it previously passed, can you factor out the common code and
> > create a new test with the additional coverage?
> >
> > i.e. the rule of thumb is that once a test is "done" we don't go
> > back and modify it in significant ways - we write a new unit test
> > that covers the new/extended functionality. Redundancy in unit tests
> > is not a bad thing...
> 
> Right. The only reason I did this, instead of a new test file, is that
> because the former fix which btrfs/030 relates to is not yet in any
> kernel release. Given this fact, what do you think?

Ok, so if it already fails for everyone, then I think we'll be fine
to modify it like this. "done" is a flexible concept when it comes
to unit tests ;)

Cheers,

Dave.

Patch
diff mbox

diff --git a/tests/btrfs/030 b/tests/btrfs/030
index 6678ed8..6b52a0d 100755
--- a/tests/btrfs/030
+++ b/tests/btrfs/030
@@ -73,22 +73,41 @@  mkdir $SCRATCH_MNT/a/b/www
 echo "hey" > $SCRATCH_MNT/a/b/foobar.txt
 mkdir -p $SCRATCH_MNT/a/b/c3/x/y
 
+mkdir -p $SCRATCH_MNT/a/b/foo1/foo2
+echo "hey" > $SCRATCH_MNT/a/b/foo1/foo2/f.txt
+mkdir $SCRATCH_MNT/a/b/foo3
+
+mkdir -p $SCRATCH_MNT/a/b/bar1/bar2/bar3/bar4
+echo "ola" > $SCRATCH_MNT/a/b/bar1/bar2/bar3/bar4/hello.txt
+
 # Directory tree looks like:
 #
-# .                         (ino 256)
-# |-- a/                    (ino 257)
-#     |-- b/                (ino 258)
-#         |-- c/            (ino 259)
-#         |   |-- file.txt  (ino 260)
-#         |   |-- d/        (ino 261)
+# .                                      (ino 256)
+# |-- a/                                 (ino 257)
+#     |-- b/                             (ino 258)
+#         |-- c/                         (ino 259)
+#         |   |-- file.txt               (ino 260)
+#         |   |-- d/                     (ino 261)
+#         |
+#         |-- c2/                        (ino 262)
+#         |-- www/                       (ino 263)
+#         |-- foobar.txt                 (ino 264)
+#         |
+#         |-- c3/                        (ino 265)
+#         |   |-- x/                     (ino 266)
+#         |       |-- y/                 (ino 267)
+#         |
+#         |-- foo1/                      (ino 268)
+#         |    |---foo2/                 (ino 269)
+#         |         |---f.txt            (ino 270)
 #         |
-#         |-- c2/           (ino 262)
-#         |-- www/          (ino 263)
-#         |-- foobar.txt    (ino 264)
+#         |-- foo3/                      (ino 271)
 #         |
-#         |-- c3/           (ino 265)
-#             |-- x/        (ino 266)
-#                 |-- y/    (ino 267)
+#         |-- bar1/                      (ino 272)
+#              |-- bar2/                 (ino 273)
+#                  |-- bar3/             (ino 274)
+#                      |-- bar4          (ino 275)
+#                           |--hello.txt (ino 276)
 
 run_check $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
     $SCRATCH_MNT/mysnap1
@@ -104,21 +123,46 @@  mv $SCRATCH_MNT/a/b/foobar.txt $SCRATCH_MNT/a/b/c2/y2/x2/qwerty.txt
 ln $SCRATCH_MNT/a/b/c2/d2/cc/file.txt $SCRATCH_MNT/a/b/c2/y2/x2/Z/file_link.txt
 mv $SCRATCH_MNT/a/b/c2/d2/cc/file.txt $SCRATCH_MNT/a/b/c2/y2/x2
 
+mv $SCRATCH_MNT/a/b/foo3 $SCRATCH_MNT/a/b/foo1/foo33
+mv $SCRATCH_MNT/a/b/foo1/foo2 $SCRATCH_MNT/a/b/foo1/foo33/foo22
+mv $SCRATCH_MNT/a/b/foo1/foo33/foo22/f.txt \
+    $SCRATCH_MNT/a/b/foo1/foo33/foo22/fff.txt
+
+echo " hello" >> $SCRATCH_MNT/a/b/bar1/bar2/bar3/bar4/hello.txt
+mv $SCRATCH_MNT/a/b/bar1/bar2/bar3/bar4/hello.txt \
+    $SCRATCH_MNT/a/b/bar1/bar2/bar3/bar4/hello2.txt
+mv $SCRATCH_MNT/a/b/bar1/bar2/bar3/bar4 $SCRATCH_MNT/a/b/k44
+mv $SCRATCH_MNT/a/b/bar1/bar2/bar3 $SCRATCH_MNT/a/b/k44
+mv $SCRATCH_MNT/a/b/bar1/bar2 $SCRATCH_MNT/a/b/k44/bar3
+mv $SCRATCH_MNT/a/b/bar1 $SCRATCH_MNT/a/b/k44/bar3/bar2/k11
+
 # Directory tree now looks like:
 #
 # .                                         (ino 256)
 # |-- a/                                    (ino 257)
 #     |-- b/                                (ino 258)
 #         |-- c2/                           (ino 262)
-#             |-- d2/                       (ino 261)
-#             |   |-- cc/                   (ino 259)
-#             |        |-- file.txt         (ino 260)
-#             |-- y2/                       (ino 267)
-#                 |-- x2/                   (ino 266)
-#                     |-- qwerty.txt        (ino 264)
-#                     |-- WWW/              (ino 263)
-#                     |-- Z/                (ino 265)
-#                         |-- file_link.txt
+#         |   |-- d2/                       (ino 261)
+#         |   |   |-- cc/                   (ino 259)
+#         |   |
+#         |   |-- y2/                       (ino 267)
+#         |       |-- x2/                   (ino 266)
+#         |           |-- file.txt          (ino 260)
+#         |           |-- qwerty.txt        (ino 264)
+#         |           |-- WWW/              (ino 263)
+#         |           |-- Z/                (ino 265)
+#         |               |-- file_link.txt
+#         |
+#         |-- foo1/                         (ino 268)
+#         |    |---foo33/                   (ino 271)
+#         |          |---foo22/             (ino 269)
+#         |                |---fff.txt      (ino 270)
+#         |
+#         |-- k44/                          (ino 275)
+#              |-- hello2.txt               (ino 276)
+#              |-- bar3/                    (ino 274)
+#                   |-- bar2/               (ino 273)
+#                        |-- k11/           (ino 272)
 
 run_check $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
     $SCRATCH_MNT/mysnap2