-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chromium: Type each character as the screen changes #15173
chromium: Type each character as the screen changes #15173
Conversation
openqa-clone-custom-git-refspec #15173 https://openqa.opensuse.org/tests/2443233 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://openqa.opensuse.org/tests/2449613 took 2m26s for the chromium test module. Your verification run took 3m44s. I don't think the test should be slowed down that significantly. Please check runtime of the module.
Any suggestions on how to avoid that while still verifying every typed character? I didn't change anything else. |
I am not sure if my observation really shows a problem. So to be sure that your changes actually cause this slow-down can you try to run multiple tests with just the "chromium" test module included to check? E.g. call The option https://gist.github.com/okurz/295e2ad3a13a37a02294f0c6c26fe69a is the section of the log where the entering and waiting happens. Just the first two characters:
the "waiting for screen change" is done every 500ms and the screen change is only detected in the third iteration. I assume here we would benefit from something similar to the "no_wait" in https://github.com/os-autoinst/os-autoinst/blob/master/testapi.pm#L378 . I suggest to check the implementation of the |
Waiting on the results of #2109 for now to confirm how to continue |
You don't mean pull request EDIT: Oh, you mean os-autoinst/os-autoinst#2109, right |
Apparently looking into optimizing the underlying code is far from trivial, so I'd like to wrap this up as-is. We know it works. See also my comment on the ticket. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As https://progress.opensuse.org/issues/109737 still doesn't have auto-review or anything let's approve as is despite the slow-down. Created a new ticket https://progress.opensuse.org/issues/114412 to optimize the performance.
See: https://progress.opensuse.org/issues/109737