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

tests: Print dots by default in functional tests #14569

Merged
merged 1 commit into from Nov 1, 2018

Conversation

Projects
None yet
8 participants
@ken2812221
Copy link
Member

commented Oct 25, 2018

In cron job (https://travis-ci.org/bitcoin/bitcoin/builds/445823485), the functional tests would fail due to silent for 10 mins.

After applying this patch, we con't see any extra characters printed on screen but also avoid timeout (https://travis-ci.org/ken2812221/bitcoin/builds/445981698)

@fanquake fanquake added the Tests label Oct 25, 2018

@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 25, 2018

For this reason the test runner prints . every so few seconds, but somehow—that's no longer enough?

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Oct 25, 2018

This is probably due to if self.logging_level == logging.DEBUG: from #14504. Imo that condition can be removed.

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2018

No more conflicts as of last run.

@ken2812221 ken2812221 changed the title travis: Print characters per 9 min to avoid timeout travis: Print empty characters per 9 min to avoid timeout Oct 27, 2018

@ken2812221 ken2812221 force-pushed the ken2812221:travis-avoid-timeout branch Oct 29, 2018

@ken2812221

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2018

@MarcoFalke Is this what you mean or just remove the if condition to always print .?

@ken2812221 ken2812221 force-pushed the ken2812221:travis-avoid-timeout branch Oct 29, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

Previously there was no condition, and it would always print: https://github.com/bitcoin/bitcoin/pull/14504/files?w=1#diff-a5b9b84e3a3387476629e74ddb227a7eR502

Not sure why that got added.

@Gnappuraz

This comment has been minimized.

Copy link
Contributor

commented Oct 29, 2018

@ken2812221 don't you have to kill the waiting at some point since you put it on background?
Also I think that travis has a 10m timeout, so printing every 9m should cut it.
I've done something like this to avoid it while running the tests...

if [ "$RUN_TESTS" = "true" ]; then
  while sleep 9m; do echo "=====[ $SECONDS seconds still running ]====="; done &
  make $MAKEJOBS check VERBOSE=1
  kill %1
fi
@isghe

This comment has been minimized.

Copy link
Contributor

commented Oct 29, 2018

Previously there was no condition, and it would always print: https://github.com/bitcoin/bitcoin/pull/14504/files?w=1#diff-a5b9b84e3a3387476629e74ddb227a7eR502

Not sure why that got added.

To be coherent with --quiet option "only print results summary and failure logs".

@ken2812221 Maybe we can add the --dots option to show the dots. So the travis command could be
DOCKER_EXEC test/functional/test_runner.py --combinedlogslen=4000 --coverage --quiet --failfast --dots ${extended};

or the --travis option.

return TestResult(name, status, int(time.time() - start_time)), testdir, stdout, stderr
if self.logging_level == logging.DEBUG:
print('.', end='', flush=True)
dot_count += 1

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Oct 29, 2018

Member

I think it was nice to have some indicator that something was running. I'd prefer to keep it.

This comment has been minimized.

Copy link
@isghe

isghe Oct 30, 2018

Contributor

I agree: running functional tests, without any progress indicator, it's not a good incentive to run functional tests.

@ken2812221 ken2812221 force-pushed the ken2812221:travis-avoid-timeout branch Oct 30, 2018

@ken2812221 ken2812221 changed the title travis: Print empty characters per 9 min to avoid timeout tests: Print dots to avoid timeout when --dots given Oct 30, 2018

@ken2812221 ken2812221 force-pushed the ken2812221:travis-avoid-timeout branch Oct 30, 2018

@Gnappuraz
Copy link
Contributor

left a comment

utACK 2e4435c

@MarcoFalke
Copy link
Member

left a comment

Sorry for nitpicking this trivial change to death. Feel free to ignore my feedback.

test/functional/test_runner.py Outdated
@@ -441,7 +443,7 @@ class TestHandler:
Trigger the test scripts passed in via the list.
"""

def __init__(self, num_tests_parallel, tests_dir, tmpdir, test_list=None, flags=None, logging_level=logging.DEBUG):
def __init__(self, num_tests_parallel, tests_dir, tmpdir, test_list=None, flags=None, logging_level=logging.DEBUG, dots=False):

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Oct 30, 2018

Member

self.logging_level is now unused. I'd slightly prefer to remove it.

test/functional/test_runner.py Outdated
@@ -225,6 +225,7 @@ def main():
parser.add_argument('--quiet', '-q', action='store_true', help='only print results summary and failure logs')
parser.add_argument('--tmpdirprefix', '-t', default=tempfile.gettempdir(), help="Root directory for datadirs")
parser.add_argument('--failfast', action='store_true', help='stop execution after the first test failure')
parser.add_argument('--dots', action='store_true', help='print dots while testing to avoid silence timeout on travis-ci.')

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Oct 30, 2018

Member

Is it really necessary to add an option to enable a feature that should be on by default? I think the average user would prefer to see dots without having to enable them by typing --dots every time.

test/functional/test_runner.py Outdated
@@ -225,6 +225,7 @@ def main():
parser.add_argument('--quiet', '-q', action='store_true', help='only print results summary and failure logs')

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Oct 30, 2018

Member
Suggested change
parser.add_argument('--quiet', '-q', action='store_true', help='only print results summary and failure logs')
parser.add_argument('--quiet', '-q', action='store_true', help='only print dots, results summary and failure logs')

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Oct 30, 2018

Member

Could just add dots here and remove the other option?

@ken2812221 ken2812221 force-pushed the ken2812221:travis-avoid-timeout branch Oct 30, 2018

@ken2812221 ken2812221 force-pushed the ken2812221:travis-avoid-timeout branch to 4bd125f Oct 30, 2018

@ken2812221 ken2812221 changed the title tests: Print dots to avoid timeout when --dots given tests: Print dots by default in functional tests Oct 31, 2018

@ken2812221

This comment has been minimized.

Copy link
Member Author

commented Oct 31, 2018

@MarcoFalke Now it print dots by default.

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

utACK 4bd125f

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 31, 2018

utACK 4bd125f

@jnewbery

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

Seems reasonable. The whole point of quiet mode was to not fill the screen with output. Now that the dots erase themselves after each test, that's no longer an issues.

Tested ACK 4bd125f

@MarcoFalke MarcoFalke merged commit 4bd125f into bitcoin:master Nov 1, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Nov 1, 2018

Merge #14569: tests: Print dots by default in functional tests
4bd125f tests: Print dots by default (Chun Kuan Lee)

Pull request description:

  In cron job (https://travis-ci.org/bitcoin/bitcoin/builds/445823485), the functional tests would fail due to silent for 10 mins.

  After applying this patch, we con't see any extra characters printed on screen but also avoid timeout (https://travis-ci.org/ken2812221/bitcoin/builds/445981698)

Tree-SHA512: c0412e171a451b27f9734311c7f063ad3fd7142087ed1e3786b4f303acaebc043f970523d6c2d4ef57ec5857040e2b6f7fd6345304353e7805d76044d317344d

@ken2812221 ken2812221 deleted the ken2812221:travis-avoid-timeout branch Nov 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.