manually set the text format to plain text in error #1546

aaronayres35 · 2021-03-19T20:26:52Z

Note I am observing a segfault when I set the slider to 0 in the example given in the issue and then close the information dialog (this occurs both with and without the changes of this PR)
Im not sure why we weren't previously using information from the pyface.api here (see: https://github.com/enthought/pyface/blob/65813858730a61f25618e98669bcc5347d44935a/pyface/message_dialog.py#L15-L43)

In any case, now in order to call setTextFormat we would not be ably to use pyface directly anyway / we can no longer call the static function QtGui.QMessageBox.information.

This is what I currently see if I set the slider to 0.0:

As opposed to this from before:

I am unsure why 2 dialogs show up (this is probably a separate bug?)

A test for this would likely require UITester + ModalDialogTester to look at the displayed text in the created modal dialog. I will investigate this now

aaronayres35 · 2021-03-19T20:41:25Z

I have the bones set up for a test, but the test hits the segfault I describe above. This will likely need some other fix

rahulporuri · 2021-03-22T09:24:18Z

Note I am observing a segfault when I set the slider to 0 in the example given in the issue and then close the information dialog (this occurs both with and without the changes of this PR)

I'm not seeing this on windows with pyqt5. What toolkit are you seeing that behavior with? And if the segfault is persistent, there is something horrible going on so that needs a separate issue.

rahulporuri · 2021-03-22T09:32:20Z

Im not sure why we weren't previously using information from the pyface.api here (see: https://github.com/enthought/pyface/blob/65813858730a61f25618e98669bcc5347d44935a/pyface/message_dialog.py#L15-L43)

I honestly dont know why I bothered but I dug into the code to realize that pyface.api.information isnt used here simply because it was added in 2008 (enthought/pyface@7aa4fb9) whereas this module was added in 2007 (266a874#diff-e282d9e0c90155c775f3886e3bd7b8d7d4f9f2a9d1b2ca7f64547d7c8a898646).

It looks like we might actually be able to replace the QMessageBox with a call to information.

rahulporuri · 2021-03-22T09:35:57Z

I am unsure why 2 dialogs show up (this is probably a separate bug?)

I'm not seeing this either. Your fix works for me on windows with python 3.6 and pyqt5 and I just see the one dialog with the correct/expected text.

aaronayres35 · 2021-03-22T18:45:05Z

Im not sure why we weren't previously using information from the pyface.api here (see: https://github.com/enthought/pyface/blob/65813858730a61f25618e98669bcc5347d44935a/pyface/message_dialog.py#L15-L43)

I honestly dont know why I bothered but I dug into the code to realize that pyface.api.information isnt used here simply because it was added in 2008 (enthought/pyface@7aa4fb9) whereas this module was added in 2007 (266a874#diff-e282d9e0c90155c775f3886e3bd7b8d7d4f9f2a9d1b2ca7f64547d7c8a898646).

It looks like we might actually be able to replace the QMessageBox with a call to information.

We cant easily use information or MessageDialog form pyface as we need direct access to the control that gets created in _create_control, which is not called until the dialog is open. But we want to set the text format before the dialog gets opened. This should be an option in pyface and I will open a separate PR there to allow specifying the text formatting option in information / MessageDialog

aaronayres35 · 2021-03-22T18:57:51Z

I am unsure why 2 dialogs show up (this is probably a separate bug?)

I'm not seeing this either. Your fix works for me on windows with python 3.6 and pyqt5 and I just see the one dialog with the correct/expected text.

I tried locally with Ubuntu and pyqt5 and everything is exactly as I would expect. It seems to be a macos specific thing

On macOS, I git cleaned and create a fresh edm env and ran an example on master. I still see 2 dialogs and the segfault when trying to close one of them. I believe there is a bug here (potentially at the qt level) with macOS that is unrelated to the changes made in this PR. I will open an issue, but I am unsure what to do with this PR for the test

aaronayres35 · 2021-03-22T21:03:56Z

I pushed hoping to see if the macOS CI would have the same issue I was seeing. Unfortunately, the appveyor build has no logs (although it did fail).

Seemingly orthogonally, the Ubuntu pyside2 GH action is failing on a completely different test (????):

est_base_url_changed (traitsui.tests.editors.test_html_editor.TestHTMLEditor) ... Command '['edm', 'run', '-e', 'traitsui-test-3.6-pyside2', '--', 'python', '-W', 'default', '-m', 'coverage', 'run', '-p', '-m', 'unittest', 'discover', '-v', 'traitsui']' returned non-zero exit status 252.
/usr/bin/bash /home/runner/work/_actions/GabrielBB/xvfb-action/v1/cleanup.sh
No xvfb processes to kill
Error: The process '/usr/bin/xvfb-run' failed with exit code 1

aaronayres35 · 2021-03-22T21:07:53Z

traitsui/tests/test_editor.py

+    def test_editor_error_msg(self):
+        from pyface.toolkit import toolkit_object
+        from pyface.qt import QtGui
+        from traits.api import HasTraits, Range
+
+        from traitsui.testing.api import (
+             KeyClick, KeySequence, Textbox, UITester
+        )
+
+        ModalDialogTester = toolkit_object(
+            "util.modal_dialog_tester:ModalDialogTester"
+        )
+
+        class Foo(HasTraits):
+            x = Range(low=0.0, high=1.0, value=0.5, exclude_low=True)
+
+        foo = Foo()
+        tester = UITester()
+        with tester.create_ui(foo) as ui:
+
+            x_range = tester.find_by_name(ui, "x")
+            x_range_textbox = x_range.locate(Textbox())
+
+            for _ in range(3):
+                x_range_textbox.perform(KeyClick('Backspace'))
+
+            x_range_textbox.perform(KeySequence('0.0'))
+
+            def trigger_error():
+                x_range_textbox.perform(KeyClick('Enter'))
+
+            def check_and_close(mdtester):
+                try:
+                    with mdtester.capture_error():
+                        self.assertTrue(
+                            mdtester.has_widget(
+                                text="The 'x' trait of a Foo instance must be "
+                                     "0.0 < a floating point number <= 1.0, "
+                                     "but a value of 0.0 <class 'float'> was "
+                                     "specified.",
+                                type_=QtGui.QMessageBox,
+                            )
+                        )
+                finally:
+                    mdtester.close(accept=True)
+                    self.assertTrue(mdtester.dialog_was_opened)
+
+            mdtester = ModalDialogTester(trigger_error)
+            mdtester.open_and_run(check_and_close)
+            self.assertTrue(mdtester.dialog_was_opened)


The level of complexity for this test now seems to far exceed what it is trying to test :/
~~Perhaps the test could be trivia and just reach into Qt and make sure the textFormat property gets set? Like I do in enthought/pyface#907~~
This test will still require ModalDialogTester as the error method on editor does not return anything. It will only be accessible via triggering the dialog to pop up. I could replace the assert statement with
self.assertEqual(mdtester.get_dialog_widget().textFormat(), QtCore.Qt.PlainText) but that doesn't really feel any better

The macOS weirdness I am seeing feels like an orthogonal problem, and I am still very unclear on what the cause is.

The macOS weirdness I am seeing feels like an orthogonal problem, and I am still very unclear on what the cause is.

Yes. Definitely something strange going on here.

jwiggins · 2021-03-23T16:06:05Z

Checking the text of the dialog in the test seems OK to me (or is that not working consistently?).

I think the bigger issue is the Qt-specific nature of the test code. I know we're not running Wx CI jobs, but it still feels a bit wrong to have a bunch of local imports and Qt specific code in a single test. Do you plan to remove the local imports before merging this?

aaronayres35 · 2021-03-23T16:15:16Z

Checking the text of the dialog in the test seems OK to me (or is that not working consistently?).

I think the bigger issue is the Qt-specific nature of the test code. I know we're not running Wx CI jobs, but it still feels a bit wrong to have a bunch of local imports and Qt specific code in a single test. Do you plan to remove the local imports before merging this?

Yeah I will clean up the imports and specify the test requires Qt specifically before merging.
But currently there is something bad happening on OSX. If you run the example are you seeing the double dialogs / possible segfault when you try to close them locally?

jwiggins · 2021-03-23T16:26:06Z

Yes, the example from the issue causes a segfault when I run it on macOS. I believe that's a separate issue.

aaronayres35 · 2021-03-23T16:50:18Z

Yes, the example from the issue causes a segfault when I run it on macOS. I believe that's a separate issue.

Thank you very much for confirming that! I think I will just skip this test on OSX on CI then and open a separate issue

…ialog tester, or are on mac os

aaronayres35 · 2021-03-23T20:47:43Z

CI is still failing on Ubuntu/Pyside2 in a seemingly unrelated test...
I am unable to reproduce the test failure locally (on both OSX and Ubuntu)

When I run the relevant test module thought I do see the following:
"Release of profile requested but WebEnginePage still not deleted. Expect troubles !"

traitsui/tests/test_editor.py

jwiggins · 2021-03-24T11:20:22Z

Given all the crazy failures you're seeing, now might be a good time to add PYTHONFAULTHANDLER=1 to the test environment.

jwiggins

Changes look OK. Anything left here?

aaronayres35 · 2021-03-24T12:59:08Z

Changes look OK. Anything left here?

I think we are good to go? I am confused why the test_base_url_changed (traitsui.tests.editors.test_html_editor.TestHTMLEditor) test failure occurred and also confused why it suddenly went away... 🤔
It had occurred in multiple previous CI runs and somehow has disappeared now without any attempt at a fix

jwiggins · 2021-03-24T13:04:59Z

Oooh boy. Intermittent failures are the best, huh?

We should probably just leave an issue and move on.

aaronayres35 · 2021-03-24T13:15:47Z

Oooh boy. Intermittent failures are the best, huh?

We should probably just leave an issue and move on.

I've opened #1551

manually set the text format to plain text in error

884e724

aaronayres35 changed the title ~~manually set the text format to plain text in error~~ [WIP] manually set the text format to plain text in error Mar 19, 2021

start of a unittest

a2b6aa0

rahulporuri self-requested a review March 22, 2021 10:43

aaronayres35 mentioned this pull request Mar 22, 2021

fix a typo from on_trait_change to observe migration #1545

Merged

aaronayres35 added 2 commits March 22, 2021 15:29

stab at a test, but it fails for me locally

24e0087

flake8

a1704bf

aaronayres35 mentioned this pull request Mar 22, 2021

Allow passing a text format to a MessageDialog enthought/pyface#907

Merged

aaronayres35 commented Mar 22, 2021

View reviewed changes

aaronayres35 mentioned this pull request Mar 23, 2021

Editor error dialog yields 2 dialogs / subsequent segfault on MacOS and Pyqt5 #1550

Closed

clean up the test imports / skip the test if we dont have qt, modal d…

1f34a59

…ialog tester, or are on mac os

jwiggins reviewed Mar 24, 2021

View reviewed changes

traitsui/tests/test_editor.py Outdated Show resolved Hide resolved

aaronayres35 added 2 commits March 24, 2021 07:33

ref issue number for why test is skipped on mac os

c3e7838

enable faulthandler in etstool test command

c83f20b

jwiggins reviewed Mar 24, 2021

View reviewed changes

aaronayres35 mentioned this pull request Mar 24, 2021

Intermittent failures in traitsui.tests.editors.test_html_editor.py #1551

Open

aaronayres35 changed the title ~~[WIP] manually set the text format to plain text in error~~ manually set the text format to plain text in error Mar 24, 2021

jwiggins approved these changes Mar 24, 2021

View reviewed changes

aaronayres35 merged commit 62033f5 into master Mar 24, 2021

aaronayres35 deleted the fix/1543-hidden-error-msg-parts branch March 24, 2021 13:21

aaronayres35 mentioned this pull request Mar 24, 2021

Update new test_editor_error_msg #1553

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

manually set the text format to plain text in error #1546

manually set the text format to plain text in error #1546

aaronayres35 commented Mar 19, 2021 •

edited by rahulporuri

aaronayres35 commented Mar 19, 2021

rahulporuri commented Mar 22, 2021

rahulporuri commented Mar 22, 2021

rahulporuri commented Mar 22, 2021

aaronayres35 commented Mar 22, 2021

aaronayres35 commented Mar 22, 2021

aaronayres35 commented Mar 22, 2021

aaronayres35 Mar 22, 2021 •

edited

jwiggins Mar 23, 2021

jwiggins commented Mar 23, 2021

aaronayres35 commented Mar 23, 2021

jwiggins commented Mar 23, 2021

aaronayres35 commented Mar 23, 2021

aaronayres35 commented Mar 23, 2021

jwiggins commented Mar 24, 2021

jwiggins left a comment

aaronayres35 commented Mar 24, 2021

jwiggins commented Mar 24, 2021

aaronayres35 commented Mar 24, 2021

manually set the text format to plain text in error #1546

manually set the text format to plain text in error #1546

Conversation

aaronayres35 commented Mar 19, 2021 • edited by rahulporuri

aaronayres35 commented Mar 19, 2021

rahulporuri commented Mar 22, 2021

rahulporuri commented Mar 22, 2021

rahulporuri commented Mar 22, 2021

aaronayres35 commented Mar 22, 2021

aaronayres35 commented Mar 22, 2021

aaronayres35 commented Mar 22, 2021

aaronayres35 Mar 22, 2021 • edited

Choose a reason for hiding this comment

jwiggins Mar 23, 2021

Choose a reason for hiding this comment

jwiggins commented Mar 23, 2021

aaronayres35 commented Mar 23, 2021

jwiggins commented Mar 23, 2021

aaronayres35 commented Mar 23, 2021

aaronayres35 commented Mar 23, 2021

jwiggins commented Mar 24, 2021

jwiggins left a comment

Choose a reason for hiding this comment

aaronayres35 commented Mar 24, 2021

jwiggins commented Mar 24, 2021

aaronayres35 commented Mar 24, 2021

aaronayres35 commented Mar 19, 2021 •

edited by rahulporuri

aaronayres35 Mar 22, 2021 •

edited