[PATCH] user32/tests: Skip monitor tests with modes larger than original.

[Rémi Bernon]

This seems to cause a lot of timeouts on the testbot, for some reason.

Signed-off-by: Rémi Bernon 
---

Clearly this is not ideal, but it looks like the timeouts weren't
completely fixed by the update. In particular the 32bit runs are still
failing.

dlls/user32/tests/monitor.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c
index 85fb2f080a7..3809221f76c 100644
--- a/dlls/user32/tests/monitor.c
+++ b/dlls/user32/tests/monitor.c
@@ -627,6 +627,12 @@ static void test_ChangeDisplaySettingsEx(void)
         dm.dmSize = sizeof(dm);
         for (mode = 0; EnumDisplaySettingsExA(devices[device].name, mode, &dm, 0); ++mode)
         {
+            if (dm.dmPelsWidth > devices[0].original_mode.dmPelsWidth || dm.dmPelsHeight > devices[0].original_mode.dmPelsHeight)
+            {
+                trace("skipping display mode %s %dx%d@%d\n", devices[device].name, dm.dmPelsWidth, dm.dmPelsHeight, dm.dmDisplayFrequency);
+                continue;
+            }
+
             dm.dmPosition = position;
             dm.dmFields |= DM_POSITION;
             res = ChangeDisplaySettingsExA(devices[device].name, &dm, NULL, CDS_RESET, NULL);

[Zhiyi Zhang]

On 8/7/20 6:35 PM, Rémi Bernon wrote:
> This seems to cause a lot of timeouts on the testbot, for some reason.
> Signed-off-by: Rémi Bernon
>
> Clearly this is not ideal, but it looks like the timeouts weren't
> completely fixed by the update. In particular the 32bit runs are still
> failing.

Hi Rémi,

You patch prompted me to look at this again and I think I found the cause this time[1]
but I can't be sure as I can't reproduce it locally. Anyway, even if this patch works,
the bug will still pop up in, for example, dxgi, d3d* tests when they change resolutions.
We should at least look into TestBot again before accepting such workarounds.

Thanks,
Zhiyi

1: https://www.winehq.org/pipermail/wine-devel/2020-August/171411.html
> dlls/user32/tests/monitor.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c
> index 85fb2f080a7..3809221f76c 100644
> --- a/dlls/user32/tests/monitor.c
> +++ b/dlls/user32/tests/monitor.c
> @@ -627,6 +627,12 @@ static void test_ChangeDisplaySettingsEx(void)
>          dm.dmSize = sizeof(dm);
>          for (mode = 0; EnumDisplaySettingsExA(devices[device].name, mode, &dm, 0); ++mode)
>          {
> -
>        if (dm.dmPelsWidth > devices[0].original_mode.dmPelsWidth || dm.dmPelsHeight > devices[0].original_mode.dmPelsHeight)
> -
>        {
> -
>            trace("skipping display mode %s %dx%d@%d\n", devices[device].name, dm.dmPelsWidth, dm.dmPelsHeight, dm.dmDisplayFrequency);
> -
>            continue;
> -
>        }
> -
>        dm.dmPosition = position;
>        dm.dmFields |= DM_POSITION;
>        res = ChangeDisplaySettingsExA(devices[device].name, &dm, NULL, CDS_RESET, NULL);

[Rémi Bernon]

On 2020-08-07 16:36, Zhiyi Zhang wrote:
> On 8/7/20 6:35 PM, Rémi Bernon wrote:
> > This seems to cause a lot of timeouts on the testbot, for some reason.
> > Signed-off-by: Rémi Bernon
> >
> > Clearly this is not ideal, but it looks like the timeouts weren't
> > completely fixed by the update. In particular the 32bit runs are still
> > failing.
>
> Hi Rémi,
>
> You patch prompted me to look at this again and I think I found the cause this time[1]
> but I can't be sure as I can't reproduce it locally. Anyway, even if this patch works,
> the bug will still pop up in, for example, dxgi, d3d* tests when they change resolutions.
> We should at least look into TestBot again before accepting such workarounds.
>
> Thanks,
> Zhiyi
>
> 1: https://www.winehq.org/pipermail/wine-devel/2020-August/171411.html
> > dlls/user32/tests/monitor.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c
> > index 85fb2f080a7..3809221f76c 100644
> > --- a/dlls/user32/tests/monitor.c
> > +++ b/dlls/user32/tests/monitor.c
> > @@ -627,6 +627,12 @@ static void test_ChangeDisplaySettingsEx(void)
> >           dm.dmSize = sizeof(dm);
> >           for (mode = 0; EnumDisplaySettingsExA(devices[device].name, mode, &dm, 0); ++mode)
> >           {
> > -
> >        if (dm.dmPelsWidth > devices[0].original_mode.dmPelsWidth || dm.dmPelsHeight > devices[0].original_mode.dmPelsHeight)
> > -
> >        {
> > -
> >            trace("skipping display mode %s %dx%d@%d\n", devices[device].name, dm.dmPelsWidth, dm.dmPelsHeight, dm.dmDisplayFrequency);
> > -
> >            continue;
> > -
> >        }
> > -
> >         dm.dmPosition = position;
> >         dm.dmFields |= DM_POSITION;
> >         res = ChangeDisplaySettingsExA(devices[device].name, &dm, NULL, CDS_RESET, NULL);

Thanks for looking into that.

I actually wanted to resend my other user32 series but I didn't want to 
get the same timeout failures that took so long the last time. I think 
it's better now but it still takes an awful lot of time to run the tests 
because of this issue.

If we can somehow temporarily downgrade the testbot it may be better, 
assuming that it's enough to fix it.

Also, I don't know the code but I still wonder if it's really worth 
trying every possible display mode?

[Zhiyi Zhang]

On 8/7/20 10:47 PM, Rémi Bernon wrote:
> On 2020-08-07 16:36, Zhiyi Zhang wrote:
> > On 8/7/20 6:35 PM, Rémi Bernon wrote:
> > > This seems to cause a lot of timeouts on the testbot, for some reason.
> > > Signed-off-by: Rémi Bernon
> > >
> > > Clearly this is not ideal, but it looks like the timeouts weren't
> > > completely fixed by the update. In particular the 32bit runs are still
> > > failing.
> >
> > Hi Rémi,
> >
> > You patch prompted me to look at this again and I think I found the cause this time[1]
> > but I can't be sure as I can't reproduce it locally. Anyway, even if this patch works,
> > the bug will still pop up in, for example, dxgi, d3d* tests when they change resolutions.
> > We should at least look into TestBot again before accepting such workarounds.
> >
> > Thanks,
> > Zhiyi
> >
> > 1: https://www.winehq.org/pipermail/wine-devel/2020-August/171411.html
> > > dlls/user32/tests/monitor.c | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > >
> > > diff --git a/dlls/user32/tests/monitor.c b/dlls/user32/tests/monitor.c
> > > index 85fb2f080a7..3809221f76c 100644
> > > --- a/dlls/user32/tests/monitor.c
> > > +++ b/dlls/user32/tests/monitor.c
> > > @@ -627,6 +627,12 @@ static void test_ChangeDisplaySettingsEx(void)
> > >           dm.dmSize = sizeof(dm);
> > >           for (mode = 0; EnumDisplaySettingsExA(devices[device].name, mode, &dm, 0); ++mode)
> > >           {
> > > +            if (dm.dmPelsWidth > devices[0].original_mode.dmPelsWidth || dm.dmPelsHeight > devices[0].original_mode.dmPelsHeight)
> > > +            {
> > > +                trace("skipping display mode %s %dx%d@%d\n", devices[device].name, dm.dmPelsWidth, dm.dmPelsHeight, dm.dmDisplayFrequency);
> > > +                continue;
> > > +            }
> > > -
> > >
> > > dm.dmPosition = position;
> > >               dm.dmFields |= DM_POSITION;
> > >               res = ChangeDisplaySettingsExA(devices[device].name, &dm, NULL, CDS_RESET, NULL);
>
> Thanks for looking into that.
>
> I actually wanted to resend my other user32 series but I didn't want to get the same timeout failures that took so long the last time. I think it's better now but it still takes an awful lot of time to run the tests because of this issue.
>
> If we can somehow temporarily downgrade the testbot it may be better, assuming that it's enough to fix it.
>
> Also, I don't know the code but I still wonder if it's really worth trying every possible display mode?

I thought about this before and decided it's best to keep them for now. The tests are still under the time limit
and I like tests to be exhaustive. For what's worth, it did expose the timeout bug, although probably not in wine.