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

PR: Close terminal tab if process was closed #98

Merged
merged 9 commits into from Jul 28, 2017

Conversation

andfoy
Copy link
Member

@andfoy andfoy commented Jul 28, 2017

Fixes #96
Fixes #95

@andfoy andfoy added this to the v0.2.1 milestone Jul 28, 2017
@andfoy andfoy self-assigned this Jul 28, 2017
def test_close_terminal_manually(qtbot):
# Setup widget
terminal = setup_terminal(qtbot)
qtbot.wait(TERM_UP)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andfoy is not there a signal that gets emitted when the terminal is ready?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that we had some mechanism in place to test when a terminal was up, and then emitted a signal when confirmed.

Copy link
Member Author

@andfoy andfoy Jul 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@goanpeca I tried to put some kind of mechanism on place, however I didn't put it because sometimes it fails. Right now I have a way to know if the prompt is ready, I'll try to use it to emit a signal.

Copy link
Member

@goanpeca goanpeca Jul 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andfoy 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@goanpeca If I use the ready signal, some tests start to fail randomly. I prefer to leave them as they are

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, then, but in this case I think is better that the test has a timer that check every 1000 ms, and we define a maximum number of retries, to account for changes in CI times etc...

@andfoy andfoy merged commit 24ad52a into spyder-ide:master Jul 28, 2017
@andfoy andfoy deleted the close_terminal branch July 28, 2017 18:34
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.

None yet

2 participants