Re: kernel32/tests: add test for RemoveDirectory

[Dmitry Timoshkov]

Zhenbo Li  wrote:
> - /* remove a nonexist directory */
> - rc = RemoveDirectoryA(directory);
> - todo_wine
> - ok(!rc && GetLastError() == ERROR_FILE_NOT_FOUND,
> -
>    "Expected ERROR_FILE_NOT_FOUND, got ret=%d, gle=%d\n", rc, GetLastError() );

This kind of test for several values simultaneously is really very awkward
as a starting point for any person who is going to work on this, or just is
looking for a simple bug to fix and greps for todo_wine in the tests. First
of all it's not clear where the failure is: is it the API failing to return
correct return value, or is it just last error which is set differently?
I admit that the error message mentions both values, but it's often more
useful to spot areas with todo_wine statements and clearly see the reason
of a failure without actually running the test.

kernel32/tests: add test for RemoveDirectory

[Zhenbo Li]

2013/12/25 Dmitry Timoshkov :
> Zhenbo Li  wrote:
> > - /* remove a nonexist directory */
> > - rc = RemoveDirectoryA(directory);
> > - todo_wine
> > - ok(!rc && GetLastError() == ERROR_FILE_NOT_FOUND,
> > -
> >    "Expected ERROR_FILE_NOT_FOUND, got ret=%d, gle=%d\n", rc, GetLastError() );
>
> This kind of test for several values simultaneously is really very awkward
> as a starting point for any person who is going to work on this, or just is
> looking for a simple bug to fix and greps for todo_wine in the tests. First
> of all it's not clear where the failure is: is it the API failing to return
> correct return value, or is it just last error which is set differently?
> I admit that the error message mentions both values, but it's often more
> useful to spot areas with todo_wine statements and clearly see the reason
> of a failure without actually running the test.
>
> --
> Dmitry.

You are right, I'll send another patch.

kernel32/tests: add test for RemoveDirectory

[刘涛]

I think that if you want GetLastError(),  "SetLastError( 0xdeadbeef );" should be added before every function which could  SetLastError().

--
Thanks，
Tao, Liu

At 2013-12-25 22:23:44,"Zhenbo Li"  wrote:
> 2013/12/25 Dmitry Timoshkov :
> > Zhenbo Li  wrote:
> > > - /* remove a nonexist directory */
> > > - rc = RemoveDirectoryA(directory);
> > > - todo_wine
> > > - ok(!rc && GetLastError() == ERROR_FILE_NOT_FOUND,
> > > -
> > >    "Expected ERROR_FILE_NOT_FOUND, got ret=%d, gle=%d\n", rc, GetLastError() );
> >
> > This kind of test for several values simultaneously is really very awkward
> > as a starting point for any person who is going to work on this, or just is
> > looking for a simple bug to fix and greps for todo_wine in the tests. First
> > of all it's not clear where the failure is: is it the API failing to return
> > correct return value, or is it just last error which is set differently?
> > I admit that the error message mentions both values, but it's often more
> > useful to spot areas with todo_wine statements and clearly see the reason
> > of a failure without actually running the test.
> >
> > --
> > Dmitry.
>
> You are right, I'll send another patch.
>
> -- 
> Have a nice day!
> Zhenbo Li

kernel32/tests: add test for RemoveDirectory

[Zhenbo Li]

2013/12/26 刘涛 :
> I think that if you want GetLastError(),  "SetLastError( 0xdeadbeef );"
> should be added before every function which could  SetLastError().

Thanks for mentioning that.
Most wine test cases don't call SetLastError, I don't know the reason,
and I just followed previous code.

BTW, in patch f39c64a85ce3617bd1008f
I've added `SetLastError(0xdeadbeef)`, but it was removed by Alexandre Julliard