Re: cmd/tests: Test del /s recursion in subdirectories with colons.
On Tue, Aug 30, 2011 at 14:19, Octavian Voicu <octavian.voicu(a)gmail.com> wrote:
-- Issue was fixed by 94d2312fe2fdd77669ac826afa24e6821571ebba. This test is to prevent future regressions, as suggested by Frederic.
Windows doesn't accept colons in directory names, so it will just fail on the mkdir/rmdir commands and not affect the tests.
--- programs/cmd/tests/test_builtins.cmd | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 1250e89..34c44f5 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -854,6 +854,7 @@ rmdir del_q_dir echo ------------ Testing del /s -------------- mkdir "foo bar" cd "foo bar" +mkdir "foo:" echo hi > file1.dat echo there > file2.dat echo bub > file3.dat @@ -868,6 +869,7 @@ for %%f in (1 2 3) do if exist file%%f.dat echo Del /s failed on file%%f for %%f in (1 2 3) do if exist file%%f.dat del file%%f.dat if exist "file with spaces.dat" echo Del /s failed on "file with spaces.dat" if exist "file with spaces.dat" del "file with spaces.dat" +rmdir "foo:" cd .. rmdir "foo bar"
You don't test the result of your 'mkdir "foo:"'. You can't know from the test whether the dir was created and removed, or never existed. You should use something like "if exist ... echo blablabla", or a 'dir /b' in a directory to verify it's not created. Be aware that error messages are generally lost/discarded, so you have to test differently
2011/8/30 Frédéric Delanoy <frederic.delanoy(a)gmail.com>:
You don't test the result of your 'mkdir "foo:"'. You can't know from the test whether the dir was created and removed, or never existed.
That's the whole point! I tested in cmd prompt on a Windows XP machine (and I assume all Windows versions act the same in this regard), and mkdir refuses to create a directory with colons in its name (same goes with explorer). Even if it might be possible to create a directory with a colon in its name by using some API, it's not relevant. So this test is strictly for Wine -- but it does have to pass on Windows too. The test I wrote is designed to fail (with a stack overflow) if you revert my earlier patch. It's not like any (sane) Windows application will be trying to create directories with colons in their names, but if it happens that the users has such a directory, you don't want Wine to go crashing because of it.
You should use something like "if exist ... echo blablabla", or a 'dir /b' in a directory to verify it's not created. Be aware that error messages are generally lost/discarded, so you have to test differently
I don't see the point of actually testing if the directory was created, because it will always fail on Windows (and you can't really mark this failure as broken, because it's standard Windows behavior). Octavian
2011/8/30 Octavian Voicu <octavian.voicu(a)gmail.com>:
2011/8/30 Frédéric Delanoy <frederic.delanoy(a)gmail.com>:
You don't test the result of your 'mkdir "foo:"'. You can't know from the test whether the dir was created and removed, or never existed.
That's the whole point! I tested in cmd prompt on a Windows XP machine (and I assume all Windows versions act the same in this regard), and mkdir refuses to create a directory with colons in its name (same goes with explorer). Even if it might be possible to create a directory with a colon in its name by using some API, it's not relevant.
OK
So this test is strictly for Wine -- but it does have to pass on Windows too. The test I wrote is designed to fail (with a stack overflow) if you revert my earlier patch. It's not like any (sane) Windows application will be trying to create directories with colons in their names, but if it happens that the users has such a directory, you don't want Wine to go crashing because of it.
You should use something like "if exist ... echo blablabla", or a 'dir /b' in a directory to verify it's not created. Be aware that error messages are generally lost/discarded, so you have to test differently
I don't see the point of actually testing if the directory was created, because it will always fail on Windows (and you can't really mark this failure as broken, because it's standard Windows behavior).
My point was simply to make sure that subsequent wine patches, maybe a year or two from now, won't make it "work" again (think regression) I never talked about marking it as broken (it obviously doesn't work on any windows), just that running a plain "mkdir foo:" and not checking the expected outcome is pointless IMHO (besides detecting crash/overflows don't occur again in wine) Maybe a simple "if exist foo: (echo wine is broken)" would be sufficient. This way you would detect quickly it wine somehow makes it "work" again. Frédéric
2011/8/31 Frédéric Delanoy <frederic.delanoy(a)gmail.com>:
My point was simply to make sure that subsequent wine patches, maybe a year or two from now, won't make it "work" again (think regression)
You got that wrong -- it *does* work on Wine already :) I think we can be more permissive in this aspect, it's not like some weird app will fail because it succeeds in creating a "foo:" directory.
I never talked about marking it as broken (it obviously doesn't work on any windows), just that running a plain "mkdir foo:" and not checking the expected outcome is pointless IMHO (besides detecting crash/overflows don't occur again in wine) Maybe a simple "if exist foo: (echo wine is broken)" would be sufficient. This way you would detect quickly it wine somehow makes it "work" again.
Mkdir / cd only fail if the 2nd char is a colon (so "a:", "a:foo", "1:" all fail), presumably because they parse that as a root drive. Mkdir "foo:" works on Wine. That was exactly the point of the patch, make sure the crash/overflow doesn't creep back in. I guess you had something else in mind when you suggested I write a test. Octavian
participants (2)
-
Frédéric Delanoy -
Octavian Voicu