Skip to content
Snippets Groups Projects

user32/tests: Cleanup some tests.

Closed Rémi Bernon requested to merge rbernon/wine:mr/user32-tests into master
2 unresolved threads

Merge request reports

Approval is optional

Closed by Rémi BernonRémi Bernon 2 years ago (Sep 12, 2022 9:21am UTC)

Merge details

  • The changes were not merged into master.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
    • Contributor

      Zebediah Figura replied on the mailing list:

      On 6/1/22 09:12, Rémi Bernon wrote:
      > It's not supposed to fail and skipping the tests simply creates false
      > successful runs.
      
      If it's not supposed to fail, shouldn't we have an ok() call checking 
      the return value?
  • Rémi Bernon changed title from user32/tests: Cleanup some msg.c tests. to user32/tests: Cleanup some tests.

    changed title from user32/tests: Cleanup some msg.c tests. to user32/tests: Cleanup some tests.

  • Rémi Bernon added 5 commits

    added 5 commits

    • ae6beb38 - user32/tests: Fix indentation to silence a warning.
    • 66a2e554 - user32/tests: Remove unnecessary test skipping checks.
    • 38504d96 - user32/tests: Remove SetForegroundWindow success checks.
    • 1e235370 - user32/tests: Check for window class registration failure normally.
    • 34aa0969 - user32/tests: Process messages while waiting in WindowFromPoint tests.

    Compare with previous version

  • Author Developer

    v2: Check SetForegroundWindow return values.

    • Afaiu SetForegroundWindow can fail on Windows if the terminal that is running the tests wasn't the foreground window when the test was started. I.e. it is some sort of focus stealing prevention.

      In the d3d tests we have a few similar tests. Those are mostly motivated by Linux WMs refusing to give us focus, not so much on Windows. There is also the case of fvwm2 or other WMs with a focus-follows-mouse setting that will make SetForegroundWindow not do what you want it to do.

    • Author Developer

      Sure, SetForegroundWindow doesn't necessarily succeeds on windows, but in the case of the tests and how they are run on windows, it is not supposed to fail.

      If it fails it means the tests will fail anyway. There have been cases like this in testbot runs, for instance when there's an unexpected topmost window (for instance the firewall warning), and it should be fixed by fixing the environment, not by skipping tests.

      On Linux there's some other issues, but they are mitigated by the flush_events calls which makes sure X11 events stabilize in the state we expect windows to be. It is not ideal, and Wine user32 and winex11 are still racy w.r.t focus and foreground, but that's something that also need to be fixed, not ignored.

    • it should be fixed by fixing the environment, not by skipping tests.

      I'm not sure I agree. The testbot isn't the only environment where people possibly run the tests, and someone new to the entire user32 thing might find environment-induced test failures a bug and not a feature.

      That said, it would be sensible to apply different rules to d3d and user32. In d3d, handling focus loss/gain is only a relatively small part, whereas it is pretty central to user32.

    • Author Developer

      I honestly don't think you can expect tests to do anything meaningful if you are doing something else at the same time in the environment they run in. Maybe that can work for a few very well isolated things, but not for user32, and definitely not for user32:win / user32:msg.

      Even if we consider it a feature we can only at best skip tests, and the unaware user would get a seemingly successful test result, and be happy with it when most tests would have been skipped.

    • Contributor

      Zebediah Figura replied on the mailing list:

      On 6/1/22 13:43, Rémi Bernon (@rbernon) wrote:
      > On Wed Jun  1 18:36:38 2022 +0000, Stefan Dösinger wrote:
      >>>
      >>> it should be fixed by fixing the environment, not by skipping tests.
      >> I'm not sure I agree. The testbot isn't the only environment where
      >> people possibly run the tests, and someone new to the entire user32
      >> thing might find environment-induced test failures a bug and not a feature.
      >> That said, it would be sensible to apply different rules to d3d and
      >> user32. In d3d, handling focus loss/gain is only a relatively small
      >> part, whereas it is pretty central to user32.
      > I honestly don't think you can expect tests to do anything meaningful if you are doing something else at the same time in the environment they run in. Maybe that can work for a few very well isolated things, but not for `user32`, and definitely not for `user32:win` / `user32:msg`.
      > 
      > Even if we consider it a feature we can only at best skip tests, and the unaware user would get a seemingly successful test result, and be happy with it when most tests would have been skipped.
      > 
      
      Is there a chance we can sidestep the issue by using a separate desktop 
      or window station?
    • Please register or sign in to reply
  • Or maybe rephrase the test failure message emphasising a possible environment problem or adding a comment that this particular check might fail if the window manager is overly aggressive with focus prevention.

    Anyway, I'll sign out of the discussion now. Since I'm not working on user32 much it is your choice. I just know I have a d3d focus test-that-can't-fail that's failing on AJ's machine that I need to fix :-( ...

  • closed

Please register or sign in to reply
Loading