Skip to content

PR: Add flaky#445

Merged
blink1073 merged 9 commits into
ipython:masterfrom
impact27:repeat_tests
Oct 9, 2019
Merged

PR: Add flaky#445
blink1073 merged 9 commits into
ipython:masterfrom
impact27:repeat_tests

Conversation

@impact27

@impact27 impact27 commented Oct 4, 2019

Copy link
Copy Markdown
Contributor

Sometimes, with setup_kernel(cmd) as client: fails. This repeats the failing tests at most 3 times.

@ccordoba12

Copy link
Copy Markdown
Member

@impact27, could you mark test_simple_print as flaky to see if it passes with multiple repetitions?

@ccordoba12 ccordoba12 mentioned this pull request Oct 4, 2019
@impact27

impact27 commented Oct 5, 2019

Copy link
Copy Markdown
Contributor Author

It works now, flaky was failing because subprocess are acting up sometimes and were not properly closed.

@blink1073

Copy link
Copy Markdown
Contributor

Perhaps instead of using flaky we should increase the timeout based on an environment variable that we can set on Travis. That would distinguish between slow starts and actual failures.

@impact27

impact27 commented Oct 5, 2019

Copy link
Copy Markdown
Contributor Author

The setup timeout is 60 seconds so I am not sure what a reasonable timeout would be.

@impact27

impact27 commented Oct 5, 2019

Copy link
Copy Markdown
Contributor Author

Wait actually it is not quite True:

while not os.path.exists(connection_file) \
         and kernel.poll() is None \
         and time.time() < tic + SETUP_TIMEOUT:
         time.sleep(0.1)

This waits for the connection file to exist but not for it to be written. Maybe the test should be: can we read the json file?

@blink1073

Copy link
Copy Markdown
Contributor

Yeah that sounds right.

@blink1073 blink1073 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks!

@blink1073 blink1073 merged commit 0faac5c into ipython:master Oct 9, 2019
@Carreau Carreau added this to the 5.4 milestone Jun 14, 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.

4 participants