diff mbox series

[v2,03/16] t7527: temporarily disable case-insensitive tests

Message ID dad079ade7f8817f235d59c5339b82aecf4a17db.1708658300.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series FSMonitor edge cases on case-insensitive file systems | expand

Commit Message

Jeff Hostetler Feb. 23, 2024, 3:18 a.m. UTC 
From: Jeff Hostetler <jeffhostetler@github.com>

Add non-existent "SKIPME" prereq to the case-insensitive tests.

The previous commit added test cases to demonstrate an error where
FSMonitor can get confused on a case-insensitive file system when the
on-disk spelling of a file or directory is wrong.  Let's disable those
tests before we incrementally teach Git to properly recognize and
handle those types of problems (so that a bisect between here and the
final commit in this patch series won't throw a false alarm).

Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
---
 t/t7527-builtin-fsmonitor.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Junio C Hamano Feb. 23, 2024, 8:17 a.m. UTC | #1 
"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Jeff Hostetler <jeffhostetler@github.com>
>
> Add non-existent "SKIPME" prereq to the case-insensitive tests.
>
> The previous commit added test cases to demonstrate an error where
> FSMonitor can get confused on a case-insensitive file system when the
> on-disk spelling of a file or directory is wrong.  Let's disable those
> tests before we incrementally teach Git to properly recognize and
> handle those types of problems (so that a bisect between here and the
> final commit in this patch series won't throw a false alarm).

You talk about bisection, but hasn't the previous step already
broken bisection without these SKIPME prerequisites?  IOW, shouldn't
this step squashed into the previous?

Also, it is much more common to replace "test_expect_success" with
"test_expect_failure" to indicate that the steps are broken.  Was
there a reason why we choose to do it differently?

> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
> ---
>  t/t7527-builtin-fsmonitor.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
> index 3d21295f789..4acb547819c 100755
> --- a/t/t7527-builtin-fsmonitor.sh
> +++ b/t/t7527-builtin-fsmonitor.sh
> @@ -1051,7 +1051,7 @@ test_expect_success 'split-index and FSMonitor work well together' '
>  #
>  # The setup is a little contrived.
>  #
> -test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
> +test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
>  	test_when_finished "stop_daemon_delete_repo subdir_case_wrong" &&
>  
>  	git init subdir_case_wrong &&
> @@ -1128,7 +1128,7 @@ test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
>  	! grep -q " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
>  '
>  
> -test_expect_success CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
> +test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
>  	test_when_finished "stop_daemon_delete_repo file_case_wrong" &&
>  
>  	git init file_case_wrong &&
Jeff Hostetler Feb. 26, 2024, 5:12 p.m. UTC | #2 
On 2/23/24 3:17 AM, Junio C Hamano wrote:
> "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Jeff Hostetler <jeffhostetler@github.com>
>>
>> Add non-existent "SKIPME" prereq to the case-insensitive tests.
>>
>> The previous commit added test cases to demonstrate an error where
>> FSMonitor can get confused on a case-insensitive file system when the
>> on-disk spelling of a file or directory is wrong.  Let's disable those
>> tests before we incrementally teach Git to properly recognize and
>> handle those types of problems (so that a bisect between here and the
>> final commit in this patch series won't throw a false alarm).
> 
> You talk about bisection, but hasn't the previous step already
> broken bisection without these SKIPME prerequisites?  IOW, shouldn't
> this step squashed into the previous?
> 
> Also, it is much more common to replace "test_expect_success" with
> "test_expect_failure" to indicate that the steps are broken.  Was
> there a reason why we choose to do it differently?

In step 2 I created test with individual step failures baked into
the "! grep -q" steps in the bottom of each test.  I didn't want a
failure in the 50-60 lines of setup code to cause a false alarm.
So the step 2 test "succeeds" by detecting that the output is
incomplete/wrong.

I wanted to use a "test_must_fail" on those individual grep lines
rather than a negated grep, but something complained about that
function only worked on "git" commands.

I added the SKIPME here in step 3 so that I could fix the series
in small steps and without worrying about which of the small steps
caused the file or directory case to stop being broken (which might
cause confusion if someone were bisecting in this part of the history.

Let me try again with the normal "test_expect_failure" in step 2,
drop step 3, and smash step 16 into step 14.  With the rearranging
that I did in V2, both directories and files should be fixed in the
same final step -- rather than in separate steps.

Thanks
Jeff


> 
>> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
>> ---
>>   t/t7527-builtin-fsmonitor.sh | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
>> index 3d21295f789..4acb547819c 100755
>> --- a/t/t7527-builtin-fsmonitor.sh
>> +++ b/t/t7527-builtin-fsmonitor.sh
>> @@ -1051,7 +1051,7 @@ test_expect_success 'split-index and FSMonitor work well together' '
>>   #
>>   # The setup is a little contrived.
>>   #
>> -test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
>> +test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
>>   	test_when_finished "stop_daemon_delete_repo subdir_case_wrong" &&
>>   
>>   	git init subdir_case_wrong &&
>> @@ -1128,7 +1128,7 @@ test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
>>   	! grep -q " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
>>   '
>>   
>> -test_expect_success CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
>> +test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
>>   	test_when_finished "stop_daemon_delete_repo file_case_wrong" &&
>>   
>>   	git init file_case_wrong &&
diff mbox series

Patch

diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 3d21295f789..4acb547819c 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -1051,7 +1051,7 @@  test_expect_success 'split-index and FSMonitor work well together' '
 #
 # The setup is a little contrived.
 #
-test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
+test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
 	test_when_finished "stop_daemon_delete_repo subdir_case_wrong" &&
 
 	git init subdir_case_wrong &&
@@ -1128,7 +1128,7 @@  test_expect_success CASE_INSENSITIVE_FS 'fsmonitor subdir case wrong on disk' '
 	! grep -q " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
 '
 
-test_expect_success CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
+test_expect_success SKIPME,CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
 	test_when_finished "stop_daemon_delete_repo file_case_wrong" &&
 
 	git init file_case_wrong &&