Re: usp10: fix tests to allow atomic patches for new implementation

[James Hawkins]

On 12/15/06, Clinton Stimpson <cjstimpson(a)utwire.net> wrote:
> Hi,
>
> Here's a patch to make unimplemented functions return E_NOTIMPL instead
> of S_OK.  Even dependent functions already return E_NOTIMPL.  Also
> fixing tests to check that the return value is S_OK before testing the
> 'out' parameters.
> This fix allows piecemeal patches to follow with implementation for each
>   function in a separate patch.
>
> Thanks,
> Clinton Stimpson
>
> ChangeLog
>      Return E_NOTIMPL where appropriate.
>      And only check out parameters when appropriate.

There are a couple things wrong with this patch.  First, you've made
it so the tests won't be run for these functions (because they don't
return S_OK as it stands).  Second, there are probably applications
that depend on these functions returning S_OK.  You're really making
it a lot harder than it should be.  Implement each function one at a
time.  If tests start to succeed, just remove the todo_wine from them.
 So the implemented function and removed/added go in one patch.  You
just do that for each function you're implementing.

-- 
James Hawkins

usp10: fix tests to allow atomic patches for new implementation

[Clinton Stimpson]

James Hawkins wrote:
> On 12/15/06, Clinton Stimpson <cjstimpson(a)utwire.net> wrote:
> > Hi,
> >
> > Here's a patch to make unimplemented functions return E_NOTIMPL instead
> > of S_OK.  Even dependent functions already return E_NOTIMPL.  Also
> > fixing tests to check that the return value is S_OK before testing the
> > 'out' parameters.
> > This fix allows piecemeal patches to follow with implementation for each
> >   function in a separate patch.
> >
> > Thanks,
> > Clinton Stimpson
> >
> > ChangeLog
> >      Return E_NOTIMPL where appropriate.
> >      And only check out parameters when appropriate.
>
> There are a couple things wrong with this patch.  First, you've made
> it so the tests won't be run for these functions (because they don't
> return S_OK as it stands).  Second, there are probably applications
> that depend on these functions returning S_OK.  You're really making
> it a lot harder than it should be.  Implement each function one at a
> time.  If tests start to succeed, just remove the todo_wine from them.
> So the implemented function and removed/added go in one patch.  You
> just do that for each function you're implementing.

Or, you guys are making it harder for me ;)

There's a
for(i ...)
  ScriptStringAnalyse
  ScriptStringCPtoX
  ScriptStringXtoCP
  todo_wine ok( compare values from ScriptStringCPtoX with 
ScriptStringXtoCP )


So.  My first patch implements ScriptStringAnalyse.  Other ScriptString* 
functions depend on that being implemented first.
So I go through and remove the todo_wine's where I get "test 
succeeded".  And the above todo_wine in the example above needs to be 
removed.  I do that, and then I get a bunch of "test failed" when i != 
0.  It passes if i =0, but it fails with i != 0.  The "make test" 
returns a failure whether I leave or remove that todo_wine.
What am I supposed to do?

Thanks,
Clinton