Audit qt dialogs connections

[ievacerny]

Part of #258

Adds additional cleanup (disconnecting qt signals) to destroy methods of some qt dialogs.

Also adds one additional smoke test for ProgressDialog.

[kitchoi]

LGTM

[ievacerny]

Something strange is happening with the tests. The first 2 runs failed because of #517. Third time I saw the same failure while the job was still running (very slowly) but now that the job is done (timed out) I no longer see the failure.

I'm sorry for going back on the review. Let's not use findChild or findChildren but use references instead and make sure the references are removed.

[ievacerny]

Something strange is happening with the tests. The first 2 runs failed because of #517. Third time I saw the same failure while the job was still running (very slowly) but now that the job is done (timed out) I no longer see the failure.

I don't think this PR is doing anything worse, quite likely it's just causing imbalance between other test interactions. So I don't think #517 shouldn't be a blocker for this and I suggest skipping the failing test until #517 is fixed.

[codecov-commenter]

Codecov Report

Merging #536 into master will increase coverage by 0.21%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #536      +/-   ##
==========================================
+ Coverage   36.99%   37.20%   +0.21%     
==========================================
  Files         470      470              
  Lines       26027    26090      +63     
  Branches     3961     3967       +6     
==========================================
+ Hits         9629     9708      +79     
+ Misses      15977    15966      -11     
+ Partials      421      416       -5     

Impacted Files	Coverage Δ
pyface/ui/qt4/about_dialog.py	84.44% <100.00%> (+2.86%)	⬆️
pyface/ui/qt4/dialog.py	94.66% <100.00%> (+0.72%)	⬆️
pyface/ui/qt4/progress_dialog.py	90.71% <100.00%> (+5.10%)	⬆️
pyface/ui/qt4/action/tool_bar_manager.py	84.28% <0.00%> (+3.92%)	⬆️
pyface/ui/qt4/action/action_item.py	46.46% <0.00%> (+5.15%)	⬆️
pyface/ui/qt4/action/menu_manager.py	70.29% <0.00%> (+7.50%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08ba6c2...fedae19. Read the comment docs.

[ievacerny]

Something strange is happening with the tests. The first 2 runs failed because of #517. Third time I saw the same failure while the job was still running (very slowly) but now that the job is done (timed out) I no longer see the failure.

I don't think this PR is doing anything worse, quite likely it's just causing imbalance between other test interactions. So I don't think #517 shouldn't be a blocker for this and I suggest skipping the failing test until #517 is fixed.

The tests finally passed, so I take my suggestion back. The problematic tests can still be skipped until the issue is fixed but that no longer needs to be done in this PR

[kitchoi]

LGTM.
Like in another PR, just one comment about removing if self.control is not None that we can discuss.

[kitchoi]

LGTM. Thank you!

[kitchoi]

Oops. The build was definitely green when this PR was merged. That was my mistake! I thought I was rebuilding master but I actually rebuilt the CI job for the PR, which then failed because the branch was deleted.

[kitchoi]

Let me see if restoring the branch could resurrect the history and mask my silly mistake...

[kitchoi]

Nope, because the merge commit is gone. Not sure how I can put the history right. My apologies!