Skip to content
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

Fixes 3941 - Do not repeatedly raise new sets of keystrokes when modals pop (stack overflow) #3949

Draft
wants to merge 7 commits into
base: v2_develop
Choose a base branch
from

Conversation

tznind
Copy link
Collaborator

@tznind tznind commented Mar 2, 2025

This PR is to explore alternative to #3946

  • Do not repeatedly raise new sets of keystrokes when modals pop and trigger run state changes.
  • Remove frame limit in v2

Outstanding Issues

This fixes the infinite loop but now with all drivers (v2 and original) the Scenarios hang on any scenario that fires up a modal dialog.

The v2 scenarios seem to pause at the end like 2s before exit - suggesting they are not getting the message to close correctly and have to wait for the timeout.

Fixes

  • Fixes #_____

Proposed Changes/Todos

  • Todo 1

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

@tznind
Copy link
Collaborator Author

tznind commented Mar 2, 2025

@tig what is supposed to exit benchmark scenarios? is demokeys supposed to send Esc after each run or is it something else? I can see the abort code but not the 'normal' exit conditions for each scenario

@tig
Copy link
Collaborator

tig commented Mar 2, 2025

There's an iteration limit.

@tig
Copy link
Collaborator

tig commented Mar 2, 2025

I added the keystroke stuff later. The original idea was to just find code that was excessively causing draws.

Then I added code to make the scenarios actually do stuff to expose more of that (demokeys).

Then I decided it would make a good benchmark framework.

It was not all that well thought through ☺️

@tig
Copy link
Collaborator

tig commented Mar 2, 2025

Oh, this made me realize : with your new driver model there ARE NO iterations if there's no need.

@tznind
Copy link
Collaborator Author

tznind commented Mar 2, 2025

There are iterations but no draws or layouts. So only thing that fires are timed events and draining input queue.

But v2 use Metrics not benchmark for tracking frequency... maybe I just need to tap into the benchmark counters too.

@tig
Copy link
Collaborator

tig commented Mar 2, 2025

FWIW, I'm intentionally re-running the Actions on all PRs to try to repro the test failures. I'm letting you know so you don't get freaked out if I quickly update these branches etc...

@tznind
Copy link
Collaborator Author

tznind commented Mar 2, 2025

ok v2 with no frame limiter seems to behave, it zips through the required iterations so fast its hard to see what is going on. There is no pause in Dialogs scenario.

WindowsDriver still runs into stop at the dialogs.

Dialogs has mix of keys - some of which should be handled in main Scenario and some get handled by the modal. But I think that is brittle and/or not working.

It used to work because every time you get run state change it raises the current demo keys again. This means when the dialog popped up it ran the Dialogs keys again which includes Enter (which would close the Dialog anyway)... that is my theory anyway.

Not sure whether v2 is going so fast it doesn't pop the Dialog in the first place or pops it fast enough to catch the requisite close events.

Is it possible to benchmark a specific scenario? Maybe verbose logging will tell us.

Also v2 seems to exit at the end early before the table gets a chance to be explored. Maybe the events to shutdown slow scenarios stays registered.

@tig
Copy link
Collaborator

tig commented Mar 2, 2025

Is it possible to benchmark a specific scenario? Maybe verbose logging will tell us.

Yea, just add the Scenario name to the UI Catalog command line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants