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 <[EMAIL PROTECTED]> 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 > >