Any thing wrong with this patch. Any Comments (expecting a lot)
On 2/17/06, Vijay Kiran Kamuju infyquest@gmail.com wrote:
Hi, I am sending the Correct Version. ChangeLog
Add More Tests for InternetQueryOption Add Tests for InternetSetOption
Thanks, Vijay On 2/17/06, Vijay Kiran Kamuju infyquest@gmail.com wrote:
please ignore this will send a correct version soon
On 2/17/06, Vijay Kiran Kamuju infyquest@gmail.com wrote:
ChangeLog
Add More Tests for InternetQueryOption Add Tests for InternetSetOption
Am Mittwoch, den 22.02.2006, 23:13 +0530 schrieb Vijay Kiran Kamuju:
Any thing wrong with this patch. Any Comments (expecting a lot)
As you asked me, i took a look on this Patch.
For my winspool-Patches, i was told to check the documented behavior and only check undocumented behavior, when an Application relies on this undocumented effect. It's of course a good Idea, to print as much Informations as possible, when a test Failed (err in your case).
When i add a new Test, I add a trace after every tested API-Call, compile the Test on linux with the mingw-crosstools as Windows-Binary (make -C dlls/your_dllname_here/tests crosstest) and run that Binary on all Windows-Versions that I have. After an update to the Parameter and the Text for ok(), I do the tests on Windows again. Then I test that Binary on wine. When there are no Failures left, i remove the traces, compile again, test again, and after a final test in wine with (make -C dlls/your_dllname_here/tests test), the diff is created.
- Your Test did not apply here, so i merged the patch by cut & paste. (malformed Patch)
- I get 5 Failures on wine. (make -C dlls/wininet/tests test)
+ hinet = InternetOpenA(useragent,INTERNET_OPEN_TYPE_DIRECT,NULL,NULL, 0); + ok((hinet != 0x0),"InternetOpen Failed\n");
- Please use (hinet != NULL) here, or is MSDN wrong (value / pointer)?
- Does the other tests work, when InternetOpenA failed, or is it more useful to return from the test here?
+ SetLastError(0xdeadbeef); + retval=InternetSetOptionA(hinet,INTERNET_OPTION_USER_AGENT, &inagent, sizeof(inagent)); + err=GetLastError(); + ok(retval == 1,"Got wrong return value %d\n",retval); + ok(err == 0xdeadbeef, "Got wrong error code %ld\n",err);
For the Example above, my Idea is: ok(retval, "returned %d with %ld (expected '!= 0')\n", retval, err);
+ len=0; + SetLastError(0xdeadbeef); + buffer=HeapAlloc(GetProcessHeap(),0,len); + retval = InternetQueryOptionW(hinet,INTERNET_OPTION_USER_AGENT,buffer,&len); + err=GetLastError(); + ok(buffer != NULL,"Output Buffer Should Not be Set"); + todo_wine ok(len == 38,"Got wrong user agent length %ld instead of %d \n",len,38); +
- You use HeapAlloc with a len of 0 and than the test "buffer != NULL": This is a Test for HeapAlloc - InternetQueryOptionW is not implemented in win95/98/me - Please avoid fixed numbers. When someone changes the Agent-Name, the complete Test must be reworked.
For the tests of the W-Functions: I was told, that i should test the Unicode-Functions only, when there are other results as from the ANSI-Version. Since our ANSI - Implementations should call the Unicode-Function, the ANSI-Test validates both Implementations. (Yes, i knew that the Implementation in wine is sometimes wrong: we have Crosscalls W->A and some Unicode-Functions are stubs while the ANSI-Versions are implemented)
+ WideCharToMultiByte(CP_ACP,0,(WCHAR *)buffer,-1,tmpbuffer,len,0,0); + todo_wine ok(!strcmp(inagent,tmpbuffer),"Got wrong user agent string % s instead of %s\n",tmpbuffer,inagent);
- This Test failed on Windows XP
+ retval = InternetSetOptionW(NULL,INTERNET_OPTION_USER_AGENT, &inpbuffer, sizeof(inpbuffer)); You use ANSI-Parameter for an UNICODE-Function.
You can take a look at the Tests for winspool/GetPrinterDriverDirectory - A test with the documented use to get the required size for the Buffer (buffersize=0) - A test with documented use (all Options valid) - Buffer is larger (must succeed) - Buffer is to small (must fail with a specific ERROR_*) (Other Functions may truncate the returned Data) - without a buffer (NULL), but with the correct buffersize. This is for testing the API, when the app was unable to allocate the Buffer, but did not check the Result of the allocation.
So there are some Reasons, why the test is not Ready yet.
I read the Docs about InternetQueryOption on MSDN and IMHO there are so many Documented values for dwOption, that I suggest to start with a testing-table and fill it with all of our implemented Options for InternetQueryOption / InternetSetOption. We have then a complete Test of our wininet-Implementation with only a small amount of code. The Table can grow very easy, when Applications use Options that we need to implement.
Thanks a lot on reviewing my patch, I have been busy lately. So i could not reply u soon enough. I will make some changes and send it to u for review.
Thanks, Vijay On 2/27/06, Detlef Riekenberg wine.dev@web.de wrote:
Am Mittwoch, den 22.02.2006, 23:13 +0530 schrieb Vijay Kiran Kamuju:
Any thing wrong with this patch. Any Comments (expecting a lot)
As you asked me, i took a look on this Patch.
For my winspool-Patches, i was told to check the documented behavior and only check undocumented behavior, when an Application relies on this undocumented effect. It's of course a good Idea, to print as much Informations as possible, when a test Failed (err in your case).
When i add a new Test, I add a trace after every tested API-Call, compile the Test on linux with the mingw-crosstools as Windows-Binary (make -C dlls/your_dllname_here/tests crosstest) and run that Binary on all Windows-Versions that I have. After an update to the Parameter and the Text for ok(), I do the tests on Windows again. Then I test that Binary on wine. When there are no Failures left, i remove the traces, compile again, test again, and after a final test in wine with (make -C dlls/your_dllname_here/tests test), the diff is created.
Your Test did not apply here, so i merged the patch by cut & paste. (malformed Patch)
I get 5 Failures on wine. (make -C dlls/wininet/tests test)
- hinet = InternetOpenA(useragent,INTERNET_OPEN_TYPE_DIRECT,NULL,NULL,
0);
- ok((hinet != 0x0),"InternetOpen Failed\n");
Please use (hinet != NULL) here, or is MSDN wrong (value / pointer)?
Does the other tests work, when InternetOpenA failed, or is it more useful to return from the test here?
- SetLastError(0xdeadbeef);
- retval=InternetSetOptionA(hinet,INTERNET_OPTION_USER_AGENT, &inagent,
sizeof(inagent));
- err=GetLastError();
- ok(retval == 1,"Got wrong return value %d\n",retval);
- ok(err == 0xdeadbeef, "Got wrong error code %ld\n",err);
For the Example above, my Idea is: ok(retval, "returned %d with %ld (expected '!= 0')\n", retval, err);
- len=0;
- SetLastError(0xdeadbeef);
- buffer=HeapAlloc(GetProcessHeap(),0,len);
- retval =
InternetQueryOptionW(hinet,INTERNET_OPTION_USER_AGENT,buffer,&len);
- err=GetLastError();
- ok(buffer != NULL,"Output Buffer Should Not be Set");
- todo_wine ok(len == 38,"Got wrong user agent length %ld instead of %d
\n",len,38);
- You use HeapAlloc with a len of 0 and than the test "buffer != NULL": This is a Test for HeapAlloc
- InternetQueryOptionW is not implemented in win95/98/me
- Please avoid fixed numbers. When someone changes the Agent-Name, the complete Test must be reworked.
For the tests of the W-Functions: I was told, that i should test the Unicode-Functions only, when there are other results as from the ANSI-Version. Since our ANSI - Implementations should call the Unicode-Function, the ANSI-Test validates both Implementations. (Yes, i knew that the Implementation in wine is sometimes wrong: we have Crosscalls W->A and some Unicode-Functions are stubs while the ANSI-Versions are implemented)
- WideCharToMultiByte(CP_ACP,0,(WCHAR *)buffer,-1,tmpbuffer,len,0,0);
- todo_wine ok(!strcmp(inagent,tmpbuffer),"Got wrong user agent string %
s instead of %s\n",tmpbuffer,inagent);
- This Test failed on Windows XP
- retval = InternetSetOptionW(NULL,INTERNET_OPTION_USER_AGENT,
&inpbuffer, sizeof(inpbuffer)); You use ANSI-Parameter for an UNICODE-Function.
You can take a look at the Tests for winspool/GetPrinterDriverDirectory
- A test with the documented use to get the required size for the Buffer (buffersize=0)
- A test with documented use (all Options valid)
- Buffer is larger (must succeed)
- Buffer is to small (must fail with a specific ERROR_*) (Other Functions may truncate the returned Data)
- without a buffer (NULL), but with the correct buffersize. This is for testing the API, when the app was unable to allocate the Buffer, but did not check the Result of the allocation.
So there are some Reasons, why the test is not Ready yet.
I read the Docs about InternetQueryOption on MSDN and IMHO there are so many Documented values for dwOption, that I suggest to start with a testing-table and fill it with all of our implemented Options for InternetQueryOption / InternetSetOption. We have then a complete Test of our wininet-Implementation with only a small amount of code. The Table can grow very easy, when Applications use Options that we need to implement.
-- By By ... ... Detlef
This is a detailed reply to ur mail.
On 2/27/06, Detlef Riekenberg wine.dev@web.de wrote:
Am Mittwoch, den 22.02.2006, 23:13 +0530 schrieb Vijay Kiran Kamuju:
Any thing wrong with this patch. Any Comments (expecting a lot)
As you asked me, i took a look on this Patch.
For my winspool-Patches, i was told to check the documented behavior and only check undocumented behavior, when an Application relies on this undocumented effect. It's of course a good Idea, to print as much Informations as possible, when a test Failed (err in your case).
When i add a new Test, I add a trace after every tested API-Call, compile the Test on linux with the mingw-crosstools as Windows-Binary (make -C dlls/your_dllname_here/tests crosstest) and run that Binary on all Windows-Versions that I have. After an update to the Parameter and the Text for ok(), I do the tests on Windows again. Then I test that Binary on wine. When there are no Failures left, i remove the traces, compile again, test again, and after a final test in wine with (make -C dlls/your_dllname_here/tests test), the diff is created.
Your Test did not apply here, so i merged the patch by cut & paste. (malformed Patch)
I get 5 Failures on wine. (make -C dlls/wininet/tests test)
- hinet = InternetOpenA(useragent,INTERNET_OPEN_TYPE_DIRECT,NULL,NULL,
0);
- ok((hinet != 0x0),"InternetOpen Failed\n");
Please use (hinet != NULL) here, or is MSDN wrong (value / pointer)?
Does the other tests work, when InternetOpenA failed, or is it more useful to return from the test here?
Just copied from http tests
- SetLastError(0xdeadbeef);
- retval=InternetSetOptionA(hinet,INTERNET_OPTION_USER_AGENT, &inagent,
sizeof(inagent));
- err=GetLastError();
- ok(retval == 1,"Got wrong return value %d\n",retval);
- ok(err == 0xdeadbeef, "Got wrong error code %ld\n",err);
For the Example above, my Idea is: ok(retval, "returned %d with %ld (expected '!= 0')\n", retval, err);
Well due our partial implementation, we have not correctly implemented returning all the error states. Hence it requires seperate tests for error messages.
- len=0;
- SetLastError(0xdeadbeef);
- buffer=HeapAlloc(GetProcessHeap(),0,len);
- retval =
InternetQueryOptionW(hinet,INTERNET_OPTION_USER_AGENT,buffer,&len);
- err=GetLastError();
- ok(buffer != NULL,"Output Buffer Should Not be Set");
- todo_wine ok(len == 38,"Got wrong user agent length %ld instead of %d
\n",len,38);
- You use HeapAlloc with a len of 0 and than the test "buffer != NULL": This is a Test for HeapAlloc
How to create a buffer with zero length does simple buffer = NULL suffice, i dont think so
- InternetQueryOptionW is not implemented in win95/98/me
Will put a check for that once the base gets thru.
- Please avoid fixed numbers. When someone changes the Agent-Name, the complete Test must be reworked.
In most other tests we use basenumbers as well, i dont prefer using other functions in it. If there is a bug in their implementation, this test get affected. Each test case should be independent and atomic
For the tests of the W-Functions: I was told, that i should test the Unicode-Functions only, when there are other results as from the ANSI-Version. Since our ANSI - Implementations should call the Unicode-Function, the ANSI-Test validates both Implementations. (Yes, i knew that the Implementation in wine is sometimes wrong: we have Crosscalls W->A and some Unicode-Functions are stubs while the ANSI-Versions are implemented)
I am testing the both Unicode and Ansi Versions. If u are concerned about SetOptionW, i have made tests for this because our implementation for it is flawed.
- WideCharToMultiByte(CP_ACP,0,(WCHAR *)buffer,-1,tmpbuffer,len,0,0);
- todo_wine ok(!strcmp(inagent,tmpbuffer),"Got wrong user agent string %
s instead of %s\n",tmpbuffer,inagent);
- This Test failed on Windows XP
Have to look into this. How to Compare char string and wchar string.
- retval = InternetSetOptionW(NULL,INTERNET_OPTION_USER_AGENT,
&inpbuffer, sizeof(inpbuffer)); You use ANSI-Parameter for an UNICODE-Function.
This is a test for bad programming ;)
You can take a look at the Tests for winspool/GetPrinterDriverDirectory
- A test with the documented use to get the required size for the Buffer (buffersize=0)
- A test with documented use (all Options valid)
- Buffer is larger (must succeed)
- Buffer is to small (must fail with a specific ERROR_*) (Other Functions may truncate the returned Data)
- without a buffer (NULL), but with the correct buffersize. This is for testing the API, when the app was unable to allocate the Buffer, but did not check the Result of the allocation.
I will look into them
So there are some Reasons, why the test is not Ready yet.
I read the Docs about InternetQueryOption on MSDN and IMHO there are so many Documented values for dwOption, that I suggest to start with a testing-table and fill it with all of our implemented Options for InternetQueryOption / InternetSetOption. We have then a complete Test of our wininet-Implementation with only a small amount of code. The Table can grow very easy, when Applications use Options that we need to implement.
I wrongly chose to implement the INTERNET_OPTION_USER_AGENT, hence i am writing the tests for those. So that my implementation gets thru. I have tests for other options also in my mind.
-- By By ... ... Detlef
Cheers, Vijay
-
Detlef Riekenberg -
Vijay Kiran Kamuju