<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">2014-01-28 Dmitry Timoshkov <span dir="ltr"><<a href="mailto:dmitry@baikal.ru" target="_blank">dmitry@baikal.ru</a>></span><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

<div id=":m9" style="overflow:hidden">Zhenbo Li <<a href="mailto:litimetal@gmail.com">litimetal@gmail.com</a>> wrote:<br>
<br>
>      ret = SHFileOperationA(&shfo);<br>
>      todo_wine<br>
>      ok(ret == 1026 ||<br>
>         ret == ERROR_FILE_NOT_FOUND || /* Vista */<br>
>         broken(ret == ERROR_SUCCESS), /* NT4 */<br>
>         "Expected 1026 or ERROR_FILE_NOT_FOUND, got %d\n", ret);<br>
> +    todo_wine<br>
> +    ok(GetLastError() == 0 ||<br>
> +        broken(GetLastError()==ERROR_FILE_NOT_FOUND), /* win 2000 */<br>
> +        "gle = %d\n", GetLastError() );<br>
<br></div></blockquote><div>Thank you for reviewing my patch.<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div id=":m9" style="overflow:hidden">


'ret == ERROR_SUCCESS (0)' case is marked as broken above, so using broken<br>
for 'GetLastError() == 0' instead of ERROR_FILE_NOT_FOUND would match that<br>
behaviour. Same comment applies for other added tests.<br>
<br></div></blockquote><div>I don't think so.<br></div><div>I think we can divide windows versions to 2 group:<br></div><div>a) 2000 or before  b) xp or later<br><br></div><div>For group a, they behave as <br>> broken(ret == ERROR_SUCCESS)<br>

</div><div>implied, but their GetLastError() were remained as ERROR_FILE_NOT_FOUND<br><br></div><div>For group b, their 'ret' value isn't 0,<br></div><div>but GetLastError() is set to 0.</div><div> <br></div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div id=":m9" style="overflow:hidden">
In general I don't think that adding last error tests for SHFileOperation<br>
is very useful because 1) this API returns error codes directly 2) according<br>
to the tests both returned error codes and set last error values differ in<br>
almost every Windows version, so it's very unlikely that there is an app<br>
that depends on this.</div></blockquote><div></div></div><br></div><div class="gmail_extra">You're right. As wine's documents[1] said,<br>> However, undocumented behavior should not be tested for unless there
        is an application<br> that relies on this behavior, and in that case the
        test should mention that application, or <br>unless one can strongly
        expect applications to rely on this behavior<br><br></div><div class="gmail_extra">There are many 'todo_wine' marks in shell32 test, and it really caused bugs(like 34324).<br></div><div class="gmail_extra">

Maybe such test cases can contribute to further development?<br></div><div class="gmail_extra"><br>[1]: <a href="http://www.winehq.org/docs/winedev-guide/testing-what">http://www.winehq.org/docs/winedev-guide/testing-what</a><br>

</div><div class="gmail_extra"><br clear="all"><br>-- <br>Have a nice day!<br>Zhenbo Li
</div></div>