PR: Enclosing quotes of sys.argv clearing instruction are now consistent #3997
Conversation
|
@andfoy, did you test this change? Is it working fine for you now? |
|
And don't worry the failing tests in Travis, that's not related to your work :-) |
|
@ccordoba12 I tested them indeed! |
|
Ok, great! |
|
lets have a hangout tomorrow please ping me |
|
@goanpeca Well, to initialize the client, it is necessary to initialize an IPython kernel, along to other variables only known to the plugin. |
Ok, in that case @ccordoba12 we will need to rework that widget logic (in another PR in the future), cause the ipython console (as a whole widget) should be self contained, no? |
| qtbot.keyClicks(client.get_control(), 'import sys;A = len(sys.argv)') | ||
| qtbot.keyPress(client.get_control(), Qt.Key_Return) | ||
| len_argv = shell.get_value("A") | ||
| assert len_argv > 0 |
There was a problem hiding this comment.
Should this be an specific value for the test?
1 or 2 or exactly 1 or 2? that would make the test more robust
There was a problem hiding this comment.
Well, it should be exactly one
There was a problem hiding this comment.
Then lets make it assert len_argv == 1
| qtbot, ipy = botipython | ||
| shell = ipy.get_current_shellwidget() | ||
| client = ipy.get_current_client() | ||
| with qtbot.waitSignal(client.shell_ready, timeout=6000) as blocker: |
There was a problem hiding this comment.
For readability lets add an empty space before the with
What @andfoy did was the right thing to do because the plugin is the one in charge of starting kernels and assigning them to clients.
Hard to do it because it needs to know about other plugins. But we could try to do it. |
|
Great good @andfoy! Thanks a lot for the IPython console tests :-) I also took the opportunity to fix some other tests that were failing locally. |
Also improve test_sys_argv_clear
ccordoba12
force-pushed
the
sys_argv_quotes
branch
from
258c7da to
d27635e
Compare
Fixes #3987