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

Improve logging of test for panel destruction #12291

Merged
merged 4 commits into from
Apr 15, 2021

Conversation

feerrenrut
Copy link
Contributor

@feerrenrut feerrenrut commented Apr 14, 2021

Link to issue number:

None

Summary of the issue:

While looking into #12220, I noticed a lot of logging noise for the tests for destruction of settings panels.

A little investigation showed that the same settings dialogs was being marked as destroyed previously because the evt.skip()
was not called.
Calling skip is not appropriate, but it is also not necessary to log that the dialog which is already marked as destoyed, will be again.

Description of how this pull request fixes the issue:

Use an enum for destroyed state, this logs better.
Only log if the dialog is not already marked as destoyed.
Improve what is logged so that the dialog is actually identifiable.

Example log message after PR:

Input: kb(desktop):escape
DEBUG - gui.settingsDialogs.SettingsDialog._setInstanceDestroyedState (18:46:00.444) - MainThread (15748):
Setting state to destroyed for instance: Input Gestures - InputGesturesDialog - <gui.inputGestures.InputGesturesDialog object at 0x1AB17DA8>
Current _instances ['NVDA Settings - CREATED', 'Input Gestures - CREATED']
IO - speech.speak (18:46:00.517) - MainThread (15748):
Speaking [LangChangeCommand ('en'), 'NVDA Settings: General (normal configuration)', 'dialog', CancellableSpeech (still valid)]
IO - speech.speak (18:46:00.519) - MainThread (15748):
Speaking [LangChangeCommand ('en'), 'Categories:', 'list', CancellableSpeech (still valid)]
IO - speech.speak (18:46:00.521) - MainThread (15748):
Speaking [LangChangeCommand ('en'), 'General', '1 of 13', CancellableSpeech (still valid)]
IO - inputCore.InputManager.executeGesture (18:46:00.802) - winInputHook (12364):
Input: kb(desktop):escape
DEBUG - gui.settingsDialogs.SettingsDialog._setInstanceDestroyedState (18:46:00.804) - MainThread (15748):
Setting state to destroyed for instance: NVDA Settings - NVDASettingsDialog - <gui.settingsDialogs.NVDASettingsDialog object at 0x1A8F8100>
Current _instances ['NVDA Settings - CREATED']

Testing strategy:

Ran locally with the NVDA settings dialog, checked log.

Known issues with pull request:

None

Change log entry:

None, these were internal constants, meant for debugging purposes only.

Code Review Checklist:

This checklist is a reminder of things commonly forgotten in a new PR.
Authors, please do a self-review and confirm you have considered the following items.
Mark items you have considered by checking them.
You can do this when editing the Pull request description with an x: [ ] becomes [x].
You can also check the checkboxes after the PR is created.

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

@AppVeyorBot

This comment has been minimized.

@feerrenrut feerrenrut merged commit 35ceac2 into master Apr 15, 2021
@feerrenrut feerrenrut deleted the improveLoggingOfPanelDestructionTests branch April 15, 2021 10:40
@nvaccessAuto nvaccessAuto added this to the 2021.1 milestone Apr 15, 2021
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.

6 participants