beyondgrep / ack2

**ack 2 is no longer being maintained. ack 3 is the latest version.**
https://github.com/beyondgrep/ack3/
Other
1.48k stars 138 forks source link

t/file-permissions.t causes a race condition in other tests #388

Closed petdance closed 10 years ago

petdance commented 10 years ago

This failure is probably caused by t/file-permission.t changing permissions on itself, and then not being able to change them back.

If t/file-permission.t wants to work on an unreadable file, it should make a temporary one and use that.

kentfredric commented 10 years ago

I'm not sure if my email is broken or yours is, but I did explain in a reply earlier (September 8) that I had concluded that was the probable cause, along with a reliable way to replicate the failure, namely:

$ prove -j2blrv  t/file-permission.t t/asp-net-ext.t

Which is why I was rather confused when you sent me a second email this week , asking me the same question again as if the first time had been forgotten =).

petdance commented 10 years ago

Aha, so it's not that the permissions weren't being set back, but that the other test wasn't expecting the t/file-permissions.t to disappear on it.

Thanks very much.

kentfredric commented 10 years ago

And I don't think its so much

and then not being able to change them back.

It does appear to change them back just fine for me.

The issue is that a secondary test runs simultaneously, and the secondary test attempts tree traversal, and then gets stuck not being able to read the file.

As long as you run additional tests after the file permission test completes, nothing bad occurs.

It just hits a race because tests other than the file permission test see the modified file in the temporary window where it is unreadable.

$ ls -la ./t/file-permission.t 
# -rw-r--r-- 1 kent kent 2445 Sep 22 08:05 ./t/file-permission.t
$ prove -blr ./t/file-permission.t 
# ./t/file-permission.t .. ok     
# All tests successful.
# Files=1, Tests=14,  0 wallclock secs ( 0.03 usr  0.01 sys +  0.27 cusr  0.04 csys =  0.35 CPU)
# Result: PASS
$ ls -la ./t/file-permission.t 
# -rw-r--r-- 1 kent kent 2445 Sep 22 08:05 ./t/file-permission.t

However, if you pause the test temporarily:

$ prove -blr ./t/file-permission.t 
# ./t/file-permission.t .. 1/14 ^Z
# [1]+  Stopped                 prove -blr ./t/file-permission.t
$ ls -la ./t/file-permission.t 
# ---------- 1 kent kent 2445 Sep 22 08:05 ./t/file-permission.t

And this permission state causes tests run in parallel with the above test to fail, which you can simulate by keeping the above test in a suspended state:

$ prove -lvr t/asp-net-ext.t 
# t/asp-net-ext.t .. 
# 1..2
# not ok 1 - Should have no output to stderr: ack --aspx -f
#  
# #   Failed test 'Should have no output to stderr: ack --aspx -f'
# #   at t/asp-net-ext.t line 21.
# #     Structures begin differing at:
# #          $got->[0] = 'ack: t/file-permission.t: cannot open file for reading'
# #     $expected->[0] = Does not exist
#     1..1
#         1..1
#         ok 1 - t/asp-net-ext.t
#     ok 1 - lists_match( t/asp-net-ext.t )
# ok 2 - sets_match( t/asp-net-ext.t )
# # Looks like you failed 1 test of 2.
#Dubious, test returned 1 (wstat 256, 0x100)
#
# Failed 1/2 subtests 
# 
# Test Summary Report
# -------------------
# t/asp-net-ext.t (Wstat: 256 Tests: 2 Failed: 1)
#   Failed test:  1
#   Non-zero exit status: 1
# Files=1, Tests=2,  0 wallclock secs ( 0.03 usr  0.01 sys +  0.13 cusr  0.02 csys =  0.19 CPU)
# Result: FAIL
petdance commented 10 years ago

This is fixed in 2.11_01. Thank you for the detective work.