Re: cabinet: Revert "cabinet: Fix for FDICopy with an empty cabinet file."

[Reece Dunn]

2008/4/26 James Hawkins <jhawkins(a)codeweavers.com>:
> Hi,
>
> The test was incorrect and failed on all platforms.  Vitaly, please
>  ask someone to test your patches on a real windows system in the
>  future if you don't have access to one.

Wouldn't it be better to actually fix the test case than revert it all together?

Just simply change line 630:

-    ok(ret, "Expected FDICopy to succeed\n");
+    ok(!ret, "Expected FDICopy to fail\n");

and remove the todo_wine. Adding a GetLastError check would be good as well.

- Reece

cabinet: Revert "cabinet: Fix for FDICopy with an empty cabinet file."

[James Hawkins]

On Sat, Apr 26, 2008 at 1:03 AM, Reece Dunn <msclrhd(a)googlemail.com> wrote:
> 2008/4/26 James Hawkins <jhawkins(a)codeweavers.com>:
> > Hi,
> >
> > The test was incorrect and failed on all platforms.  Vitaly, please
> >  ask someone to test your patches on a real windows system in the
> >  future if you don't have access to one.
>
> Wouldn't it be better to actually fix the test case than revert it all together?
>
> Just simply change line 630:
>
> -    ok(ret, "Expected FDICopy to succeed\n");
>  +    ok(!ret, "Expected FDICopy to fail\n");
>
> and remove the todo_wine. Adding a GetLastError check would be good as well.

Have you actually read the patch or the changelog for that matter?

-- 
James Hawkins

cabinet: Revert "cabinet: Fix for FDICopy with an empty cabinet file."

[Reece Dunn]

2008/4/26 James Hawkins <truiken(a)gmail.com>:
> On Sat, Apr 26, 2008 at 1:03 AM, Reece Dunn <msclrhd(a)googlemail.com> wrote:
> > 2008/4/26 James Hawkins <jhawkins(a)codeweavers.com>:
> > > Hi,
> > >
> > > The test was incorrect and failed on all platforms.  Vitaly, please
> > >  ask someone to test your patches on a real windows system in the
> > >  future if you don't have access to one.
> >
> > Wouldn't it be better to actually fix the test case than revert it all together?
>
> Have you actually read the patch or the changelog for that matter?

I misread the "Revert ..." part to mean a full revert of the original patch.

Sorry for the noise.
- Reece