This is a detailed reply to ur mail. 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? > 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