xfstests: deprecate busted log printing tests
RFC PATCH 1/1 xfstests: deprecate busted log printing tests
Tests 018, 081 and 082 read the contents of the log and assume that the contents will always be the same. They are trying to ensure that the contents of the log don't change for a given fixed load.
This has several problems - high level changes to the filesystem and VFS code can change the order and contents of the log. Changes to the way we sync the filesystem will change the contents of the log. background writeback occurring in the middle of the test will change the contents of the log by allowing the tail to move. Even changes to the default mkfs parameters can break them!
The tests also assume that unmount leaves a dirty log behind. We've fixed lots of problems in sync and the unmount paths over recent times, so now a clean unmount leaves a clean log behind. That is, there is nothing left in the log print output for these tests to check. IOWs, major surgery is required for these tests to be returned to their former break-when-something-changes behaviour.
However, these tests are a maintenance nightmare. They spend more time broken and failing than they do passing, and then it's not long before they get broken again. They have to cover all sorts of different permutations of log configurations and that will continue to grow and increase the complexity of making these tests continue to work. And to top it all off, I can't remember a bug actually ever being found by these tests. Hence I think we should just stop using them altogether.
So this patch deprecates 018, 081 and 082 rather than fixes them. It introduces a "deprecated" test group and puts them in it. That means the tests can still be run on older systems where they may have some use, but will not be run automatically any more, nor will any attempt be made to keep them up to date or working.
--- group | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-)
group +7 -3
@@ -104,6 +104,10 @@ atime # Test preallocation calls prealloc +# Old tests that we won't spend any effort trying to run and make work +# on current systems +deprecated + # # test-group association ... one line per test #
@@ -124,7 +128,7 @@ prealloc 015 other auto quick 016 rw auto quick 017 mount auto quick -018 log logprint v2log auto quick +018 deprecated # log logprint v2log 019 mkfs auto quick 020 metadata attr udf auto quick 021 db attr auto quick
@@ -187,8 +191,8 @@ prealloc 078 growfs auto quick 079 acl attr ioctl metadata auto quick 080 rw ioctl -081 log logprint quota auto quick -082 log logprint v2log auto quick +081 deprecated # log logprint quota +082 deprecated # log logprint v2log 083 rw auto 084 ioctl rw auto 085 log auto quick
Signature · 6 lines
1.6.5 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 27 lines · Dave Chinner
Tests 018, 081 and 082 read the contents of the log and assume that the contents will always be the same. They are trying to ensure that the contents of the log don't change for a given fixed load.
This has several problems - high level changes to the filesystem and VFS code can change the order and contents of the log. Changes to the way we sync the filesystem will change the contents of the log. background writeback occurring in the middle of the test will change the contents of the log by allowing the tail to move. Even changes to the default mkfs parameters can break them!
The tests also assume that unmount leaves a dirty log behind. We've fixed lots of problems in sync and the unmount paths over recent times, so now a clean unmount leaves a clean log behind. That is, there is nothing left in the log print output for these tests to check. IOWs, major surgery is required for these tests to be returned to their former break-when-something-changes behaviour.
However, these tests are a maintenance nightmare. They spend more time broken and failing than they do passing, and then it's not long before they get broken again. They have to cover all sorts of different permutations of log configurations and that will continue to grow and increase the complexity of making these tests continue to work. And to top it all off, I can't remember a bug actually ever being found by these tests. Hence I think we should just stop using them altogether.
Quoted 5 lines
So this patch deprecates 018, 081 and 082 rather than fixes them. It introduces a "deprecated" test group and puts them in it. That means the tests can still be run on older systems where they may have some use, but will not be run automatically any more, nor will any attempt be made to keep them up to date or working.
Sounds fine to me. Btw, at some point we should switch check to only run the auto group by default.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 27 lines · Dave Chinner
Tests 018, 081 and 082 read the contents of the log and assume that the contents will always be the same. They are trying to ensure that the contents of the log don't change for a given fixed load.
This has several problems - high level changes to the filesystem and VFS code can change the order and contents of the log. Changes to the way we sync the filesystem will change the contents of the log. background writeback occurring in the middle of the test will change the contents of the log by allowing the tail to move. Even changes to the default mkfs parameters can break them!
The tests also assume that unmount leaves a dirty log behind. We've fixed lots of problems in sync and the unmount paths over recent times, so now a clean unmount leaves a clean log behind. That is, there is nothing left in the log print output for these tests to check. IOWs, major surgery is required for these tests to be returned to their former break-when-something-changes behaviour.
However, these tests are a maintenance nightmare. They spend more time broken and failing than they do passing, and then it's not long before they get broken again. They have to cover all sorts of different permutations of log configurations and that will continue to grow and increase the complexity of making these tests continue to work. And to top it all off, I can't remember a bug actually ever being found by these tests. Hence I think we should just stop using them altogether.
Quoted 5 lines
So this patch deprecates 018, 081 and 082 rather than fixes them. It introduces a "deprecated" test group and puts them in it. That means the tests can still be run on older systems where they may have some use, but will not be run automatically any more, nor will any attempt be made to keep them up to date or working.
I think this is a reasonable thing to do. My only thought is that it might be good to somehow indicate *when* or *why* something got deprecated. I.e., suppose a new feature BLAH_ASYNC makes a test no longer produce output in a well-defined order. It might be nice if, along with "deprecated" it was somehow indicated that the absense of BLAH_ASYNC allows the test to still function.
Having said that, it sounds too complicated to me. If it becomes a real issue we can address it, for starters by just documenting it in the comments or something.
-Alex
Quoted 30 lines
Signed-off-by Dave Chinner <[email protected]>--- group | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-)group +5
-1@@ -104,6 +104,10 @@ atime # Test preallocation calls prealloc +# Old tests that we won't spend any effort trying to run and make work +# on current systems +deprecated + # # test-group association ... one line per test #@@ -124,7 +128,7 @@ prealloc 015 other auto quick 016 rw auto quick 017 mount auto quick -018 log logprint v2log auto quick +018 deprecated # log logprint v2log 019 mkfs auto quick 020 metadata attr udf auto quick 021 db attr auto quick
group prealloc · line 198
Quoted 10 lines
group +2
-2@@ -187,8 +191,8 @@ prealloc 078 growfs auto quick 079 acl attr ioctl metadata auto quick 080 rw ioctl -081 log logprint quota auto quick -082 log logprint v2log auto quick +081 deprecated # log logprint quota +082 deprecated # log logprint v2log 083 rw auto 084 ioctl rw auto 085 log auto quick
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 34 lines · Alex Elder
Quoted 27 lines · Dave Chinner
Tests 018, 081 and 082 read the contents of the log and assume that the contents will always be the same. They are trying to ensure that the contents of the log don't change for a given fixed load.
This has several problems - high level changes to the filesystem and VFS code can change the order and contents of the log. Changes to the way we sync the filesystem will change the contents of the log. background writeback occurring in the middle of the test will change the contents of the log by allowing the tail to move. Even changes to the default mkfs parameters can break them!
The tests also assume that unmount leaves a dirty log behind. We've fixed lots of problems in sync and the unmount paths over recent times, so now a clean unmount leaves a clean log behind. That is, there is nothing left in the log print output for these tests to check. IOWs, major surgery is required for these tests to be returned to their former break-when-something-changes behaviour.
However, these tests are a maintenance nightmare. They spend more time broken and failing than they do passing, and then it's not long before they get broken again. They have to cover all sorts of different permutations of log configurations and that will continue to grow and increase the complexity of making these tests continue to work. And to top it all off, I can't remember a bug actually ever being found by these tests. Hence I think we should just stop using them altogether.
Quoted 5 lines
So this patch deprecates 018, 081 and 082 rather than fixes them. It introduces a "deprecated" test group and puts them in it. That means the tests can still be run on older systems where they may have some use, but will not be run automatically any more, nor will any attempt be made to keep them up to date or working.
Quoted 3 lines
I think this is a reasonable thing to do. My only thought is that it might be good to somehow indicate *when* or *why* something got deprecated.
That is what the commit message is for ;)
i.e. use git blame to find the commit that marked a test deprecated and then go read the commit message, which in this case would be all of the above...
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs