Re: [PATCH 1/6]: msvcp110: Add tr2_sys__Copy_file implementation and test.
On 06/27/15 19:49, YongHaoHu wrote:
+ ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, tests[i].fail_if_exists); + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test %d expect: %d, got %d\n", i+1, GetLastError(), ret); + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d errno expect 0xdeadbeef, got %d\n", i+1, errno); Please check the return value of the function explicitly. Currently your implementation returns error while it should succeed and it's not visible in the tests.
Thanks, Piotr
Hi, Piotr. On 15/6/29 下午5:29, Piotr Caban wrote:
On 06/27/15 19:49, YongHaoHu wrote:
+ ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, tests[i].fail_if_exists); + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test %d expect: %d, got %d\n", i+1, GetLastError(), ret); + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d errno expect 0xdeadbeef, got %d\n", i+1, errno); Please check the return value of the function explicitly. Currently your implementation returns error while it should succeed and it's not visible in the tests. Because the last error handling is not correct in CopyFile[1], I check GetLastError() instead of values. Do you have any good idea?
On 06/29/15 16:40, YongHaoHu wrote:
On 15/6/29 下午5:29, Piotr Caban wrote:
On 06/27/15 19:49, YongHaoHu wrote:
+ ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, tests[i].fail_if_exists); + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test %d expect: %d, got %d\n", i+1, GetLastError(), ret); + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d errno expect 0xdeadbeef, got %d\n", i+1, errno); Please check the return value of the function explicitly. Currently your implementation returns error while it should succeed and it's not visible in the tests. Because the last error handling is not correct in CopyFile[1], I check GetLastError() instead of values. Do you have any good idea? If testing exact return value of function is hard please at least test if function succeeds/fails when it should. Currently your implementation sometimes returns error even so it succeeds.
Thanks, Piotr
On 15/6/29 下午10:51, Piotr Caban wrote:
On 06/29/15 16:40, YongHaoHu wrote:
On 15/6/29 下午5:29, Piotr Caban wrote:
On 06/27/15 19:49, YongHaoHu wrote:
+ ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, tests[i].fail_if_exists); + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test %d expect: %d, got %d\n", i+1, GetLastError(), ret); + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d errno expect 0xdeadbeef, got %d\n", i+1, errno); Please check the return value of the function explicitly. Currently your implementation returns error while it should succeed and it's not visible in the tests. Because the last error handling is not correct in CopyFile[1], I check GetLastError() instead of values. Do you have any good idea? If testing exact return value of function is hard please at least test if function succeeds/fails when it should. Sorry, could you explain a bit more details? Currently your implementation sometimes returns error even so it succeeds.
Which one do you mean ? As you know, last_error hadling is not correct in wine's CopyFile, was this happened in Wine platform? Thank you very much. : P
On 06/29/15 19:14, YongHaoHu wrote:
On 15/6/29 下午10:51, Piotr Caban wrote:
On 06/29/15 16:40, YongHaoHu wrote:
On 15/6/29 下午5:29, Piotr Caban wrote:
On 06/27/15 19:49, YongHaoHu wrote:
+ ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, tests[i].fail_if_exists); + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test %d expect: %d, got %d\n", i+1, GetLastError(), ret); + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d errno expect 0xdeadbeef, got %d\n", i+1, errno); Please check the return value of the function explicitly. Currently your implementation returns error while it should succeed and it's not visible in the tests. Because the last error handling is not correct in CopyFile[1], I check GetLastError() instead of values. Do you have any good idea? If testing exact return value of function is hard please at least test if function succeeds/fails when it should. Sorry, could you explain a bit more details?
Please just check if function returns 0 (ERROR_SUCCESS) or something else. This way you don't need to check exact error value (only check if it's 0 or something else).
Currently your implementation sometimes returns error even so it succeeds.
Which one do you mean ? As you know, last_error hadling is not correct in wine's CopyFile, was this happened in Wine platform? { "f1", "tr2_test_dir\\f1_copy", TRUE } Your implementation returns error if f1_copy file already exists. It only makes sense to use GetLastError if function have failed.
Thanks, Piotr
Hi, Piotr. On 15/6/30 下午8:25, Piotr Caban wrote:
Currently your implementation sometimes returns error even so it succeeds.
Which one do you mean ? As you know, last_error hadling is not correct in wine's CopyFile, was this happened in Wine platform? { "f1", "tr2_test_dir\\f1_copy", TRUE } Your implementation returns error if f1_copy file already exists. It only makes sense to use GetLastError if function have failed.
I had tested the ret and the last_error before in testbot. The result shows that the function's return value is exactly the same as GetLastError(). ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, tests[i].fail_if_exists); printf("ret is %d\n", ret); printf("GetLastError is %d\n\n", GetLastError()); Here is the test result: https://testbot.winehq.org/JobDetails.pl?Key=14885&log_101=1#k101 And { "f1", "tr2_test_dir\\f1_copy", TRUE } returns 80( ERROR_FILE_EXSITS).
Hi, On 07/01/15 07:08, YongHaoHu wrote:
On 15/6/30 下午8:25, Piotr Caban wrote:
Currently your implementation sometimes returns error even so it succeeds.
Which one do you mean ? As you know, last_error hadling is not correct in wine's CopyFile, was this happened in Wine platform? { "f1", "tr2_test_dir\\f1_copy", TRUE } Your implementation returns error if f1_copy file already exists. It only makes sense to use GetLastError if function have failed.
I had tested the ret and the last_error before in testbot. The result shows that the function's return value is exactly the same as GetLastError(). ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, tests[i].fail_if_exists); printf("ret is %d\n", ret); printf("GetLastError is %d\n\n", GetLastError());
Here is the test result: https://testbot.winehq.org/JobDetails.pl?Key=14885&log_101=1#k101 And { "f1", "tr2_test_dir\\f1_copy", TRUE } returns 80( ERROR_FILE_EXSITS).
Sorry, I thought about this test: { "f1", "tr2_test_dir\\f1_copy", FALSE }, your implementation returns error while on windows it returns 0. Thanks, Piotr
participants (2)
-
Piotr Caban -
YongHaoHu