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

Fix tests leaking readline history #384

Merged
merged 5 commits into from
Dec 11, 2017

Conversation

deivid-rodriguez
Copy link
Owner

Fixes @MSP-Greg's comment:

FYI, test times seem to be jumping around quite a bit. Seems that one of the test files is leaving artifacts that slow successive tests? As shown below:

2017-09-25_60014 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 132 sec
2017-10-01_60083 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 25 sec
2017-10-15_60184 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 144 sec
2017-11-01_60599 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 42 sec
2017-11-07_60677 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 112 sec
2017-11-09_60723 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 27 sec
2017-11-11_60742 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 89 sec
2017-11-13_60753 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 64 sec
2017-11-14_60760 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 33 sec
2017-11-14_60762 464 runs, 527 assertions, 0 failures, 0 errors, 3 skips 26 sec

2017-11-15_60767 BAD Stopped Byebug::ThreadTest
2017-12-09_61095_2 BAD Stopped Byebug::ThreadTest

@deivid-rodriguez deivid-rodriguez force-pushed the fix/tests_leaking_readline_hist branch from 5d39e28 to 7f7e66d Compare December 10, 2017 23:10
Script processor would fail to clear Readline::HISTORY so every test was
loading a big amount of commands to and from history.
@deivid-rodriguez deivid-rodriguez force-pushed the fix/tests_leaking_readline_hist branch from 7f7e66d to 25a868c Compare December 10, 2017 23:12
@deivid-rodriguez
Copy link
Owner Author

@MSP-Greg Can you confirm this stabilizes test times for you?

@MSP-Greg
Copy link
Collaborator

@deivid-rodriguez

Yes, it does. Ran a few without logging info, they were all below 30 or so, then I decided to log them, shown below on the right is test time after the pr:

2017-09-25_60014   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips  132 sec   32
2017-10-01_60083   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips   25 sec
2017-10-15_60184   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips  144 sec   22
2017-11-01_60599   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips   42 sec
2017-11-07_60677   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips  112 sec   15
2017-11-09_60723   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips   27 sec
2017-11-11_60742   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips   89 sec   17
2017-11-13_60753   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips   64 sec   21
2017-11-14_60760   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips   33 sec
2017-11-14_60762   464 runs, 527 assertions, 0 failures, 0 errors, 3 skips   26 sec   24

465 runs, 528 assertions, 0 failures, 0 errors, 3 skips  -- NEW result

@deivid-rodriguez
Copy link
Owner Author

Yay, much better! Thanks for testing and letting me know!

@deivid-rodriguez deivid-rodriguez merged commit a664bd3 into master Dec 11, 2017
@deivid-rodriguez deivid-rodriguez deleted the fix/tests_leaking_readline_hist branch December 11, 2017 01:44
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.

2 participants