From 33068c29941eb4f438bfb8c121b2867a48c023b8 Mon Sep 17 00:00:00 2001 From: Leopold Talirz Date: Thu, 14 Nov 2019 13:02:28 +0100 Subject: [PATCH 1/2] Homogenize output of `verdi computer test` * fix indentation * add color to results of all 3 tests * add "success" to final message * add `aiida.cmdline.utils.echo.echo_highlight` function --- aiida/cmdline/commands/cmd_computer.py | 40 +++++++++++++----------- aiida/cmdline/utils/echo.py | 43 +++++++++++++++++++++----- 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/aiida/cmdline/commands/cmd_computer.py b/aiida/cmdline/commands/cmd_computer.py index 44f1c95371..19271fc18d 100644 --- a/aiida/cmdline/commands/cmd_computer.py +++ b/aiida/cmdline/commands/cmd_computer.py @@ -63,7 +63,8 @@ def _computer_test_get_jobs(transport, scheduler, authinfo): # pylint: disable= """ echo.echo('> Getting job list...') found_jobs = scheduler.get_jobs(as_dict=True) - echo.echo(' `-> OK, {} jobs found in the queue.'.format(len(found_jobs))) + echo.echo(' [{} jobs found in the queue]'.format(len(found_jobs))) + click.secho(' [OK]', fg=echo.COLORS['success'], bold=echo.BOLD) return True @@ -119,7 +120,7 @@ def _computer_test_no_unexpected_output(transport, scheduler, authinfo): # pyli ) return False - echo.echo(' [OK]') + click.secho(' [OK]', fg=echo.COLORS['success'], bold=echo.BOLD) return True @@ -141,11 +142,11 @@ def _computer_create_temp_file(transport, scheduler, authinfo): # pylint: disab file_content = "Test from 'verdi computer test' on {}".format(datetime.datetime.now().isoformat()) echo.echo('> Creating a temporary file in the work directory...') - echo.echo(' `-> Getting the remote user name...') + echo.echo(' -> Getting the remote user name...') remote_user = transport.whoami() - echo.echo(' [remote username: {}]'.format(remote_user)) + echo.echo(' [remote username: {}]'.format(remote_user)) workdir = authinfo.get_workdir().format(username=remote_user) - echo.echo(' [Checking/creating work directory: {}]'.format(workdir)) + echo.echo(' -> Checking/creating work directory: {}'.format(workdir)) try: transport.chdir(workdir) @@ -155,18 +156,18 @@ def _computer_create_temp_file(transport, scheduler, authinfo): # pylint: disab with tempfile.NamedTemporaryFile(mode='w+') as tempf: fname = os.path.split(tempf.name)[1] - echo.echo(' `-> Creating the file {}...'.format(fname)) + echo.echo(' -> Creating the file {}...'.format(fname)) remote_file_path = os.path.join(workdir, fname) tempf.write(file_content) tempf.flush() transport.putfile(tempf.name, remote_file_path) - echo.echo(' `-> Checking if the file has been created...') + echo.echo(' -> Checking if the file has been created...') if not transport.path_exists(remote_file_path): echo.echo_error('* ERROR! The file was not found!') return False - echo.echo(' [OK]') - echo.echo(' `-> Retrieving the file and checking its content...') + echo.echo(' [OK]') + echo.echo(' -> Retrieving the file and checking its content...') handle, destfile = tempfile.mkstemp() os.close(handle) @@ -174,7 +175,7 @@ def _computer_create_temp_file(transport, scheduler, authinfo): # pylint: disab transport.getfile(remote_file_path, destfile) with io.open(destfile, encoding='utf8') as dfile: read_string = dfile.read() - echo.echo(' [Retrieved]') + echo.echo(' [Retrieved]') if read_string != file_content: echo.echo_error('* ERROR! The file content is different from what was expected!') echo.echo('** Expected:') @@ -183,13 +184,14 @@ def _computer_create_temp_file(transport, scheduler, authinfo): # pylint: disab echo.echo(read_string) return False - echo.echo(' [Content OK]') + echo.echo(' [Content OK]') finally: os.remove(destfile) - echo.echo(' `-> Removing the file...') + echo.echo(' -> Removing the file...') transport.remove(remote_file_path) - echo.echo(' [Deleted successfully]') + echo.echo(' [Deleted successfully]') + click.secho(' [OK]', fg=echo.COLORS['success'], bold=echo.BOLD) return True @@ -483,7 +485,7 @@ def computer_test(user, print_traceback, computer): if user is None: user = orm.User.objects.get_default() - echo.echo('Testing computer<{}> for user<{}>...'.format(computer.name, user.email)) + echo.echo_info('Testing computer<{}> for user<{}>...'.format(computer.name, user.email)) try: authinfo = computer.get_authinfo(user) @@ -502,10 +504,12 @@ def computer_test(user, print_traceback, computer): num_tests = 0 try: - echo.echo('> Testing connection...') + echo.echo('> Testing connection') with transport: - scheduler.set_transport(transport) + click.secho(' [OK]', fg=echo.COLORS['success'], bold=echo.BOLD) num_tests += 1 + + scheduler.set_transport(transport) for test in [_computer_test_no_unexpected_output, _computer_test_get_jobs, _computer_create_temp_file]: num_tests += 1 try: @@ -526,9 +530,9 @@ def computer_test(user, print_traceback, computer): num_failures += 1 if num_failures: - echo.echo('Some tests failed! ({} out of {} failed)'.format(num_failures, num_tests)) + echo.echo_warning('Some tests failed! ({} out of {} failed)'.format(num_failures, num_tests)) else: - echo.echo('Test completed (all {} tests succeeded)'.format(num_tests)) + echo.echo_success('All {} tests succeeded)'.format(num_tests)) except Exception as error: # pylint:disable=broad-except echo.echo_error('** Error while trying to connect to the computer! Cannot perform following tests, stopping.') if print_traceback: diff --git a/aiida/cmdline/utils/echo.py b/aiida/cmdline/utils/echo.py index 40a9b33e31..ee9178d553 100644 --- a/aiida/cmdline/utils/echo.py +++ b/aiida/cmdline/utils/echo.py @@ -19,7 +19,10 @@ import click -__all__ = ('echo', 'echo_info', 'echo_success', 'echo_warning', 'echo_error', 'echo_critical', 'echo_dictionary') +__all__ = ( + 'echo', 'echo_info', 'echo_success', 'echo_warning', 'echo_error', 'echo_critical', 'echo_highlight', + 'echo_dictionary' +) # pylint: disable=too-few-public-methods @@ -31,6 +34,18 @@ class ExitCode(IntEnum): SUCCESS = 0 +COLORS = { + 'success': 'green', + 'highlight': 'green', + 'info': 'blue', + 'warning': 'bright_yellow', + 'error': 'red', + 'critical': 'red', + 'deprecated': 'red', +} +BOLD = True # whether colors are used together with 'bold' + + # pylint: disable=invalid-name def echo(message, bold=False, nl=True, err=False): """ @@ -53,7 +68,7 @@ def echo_info(message, bold=False, nl=True, err=False): :param nl: whether to print a newline at the end of the message :param err: whether to print to stderr """ - click.secho('Info: ', fg='blue', bold=True, nl=False, err=err) + click.secho('Info: ', fg=COLORS['info'], bold=True, nl=False, err=err) click.secho(message, bold=bold, nl=nl, err=err) @@ -67,7 +82,7 @@ def echo_success(message, bold=False, nl=True, err=False): :param nl: whether to print a newline at the end of the message :param err: whether to print to stderr """ - click.secho('Success: ', fg='green', bold=True, nl=False, err=err) + click.secho('Success: ', fg=COLORS['success'], bold=True, nl=False, err=err) click.secho(message, bold=bold, nl=nl, err=err) @@ -80,7 +95,7 @@ def echo_warning(message, bold=False, nl=True, err=False): :param nl: whether to print a newline at the end of the message :param err: whether to print to stderr """ - click.secho('Warning: ', fg='bright_yellow', bold=True, nl=False, err=err) + click.secho('Warning: ', fg=COLORS['warning'], bold=True, nl=False, err=err) click.secho(message, bold=bold, nl=nl, err=err) @@ -93,7 +108,7 @@ def echo_error(message, bold=False, nl=True, err=True): :param nl: whether to print a newline at the end of the message :param err: whether to print to stderr """ - click.secho('Error: ', fg='red', bold=True, nl=False, err=err) + click.secho('Error: ', fg=COLORS['error'], bold=True, nl=False, err=err) click.secho(message, bold=bold, nl=nl, err=err) @@ -111,11 +126,23 @@ def echo_critical(message, bold=False, nl=True, err=True): :param nl: whether to print a newline at the end of the message :param err: whether to print to stderr """ - click.secho('Critical: ', fg='red', bold=True, nl=False, err=err) + click.secho('Critical: ', fg=COLORS['critical'], bold=True, nl=False, err=err) click.secho(message, bold=bold, nl=nl, err=err) sys.exit(ExitCode.CRITICAL) +def echo_highlight(message, nl=True, bold=True, color='highlight'): + """ + Print a highlighted message to stdout + + :param message: the string representing the message to print + :param bold: whether to print the message in bold + :param nl: whether to print a newline at the end of the message + :param color: a color from COLORS + """ + click.secho(message, bold=bold, nl=nl, fg=COLORS[color]) + + # pylint: disable=redefined-builtin def echo_deprecated(message, bold=False, nl=True, err=True, exit=False): """ @@ -130,7 +157,7 @@ def echo_deprecated(message, bold=False, nl=True, err=True, exit=False): :param err: whether to print to stderr :param exit: whether to exit after printing the message """ - click.secho('Deprecated: ', fg='red', bold=True, nl=False, err=err) + click.secho('Deprecated: ', fg=COLORS['deprecated'], bold=True, nl=False, err=err) click.secho(message, bold=bold, nl=nl, err=err) if exit: @@ -159,7 +186,7 @@ def echo_formatted_list(collection, attributes, sort=None, highlight=None, hide= values = [getattr(entry, attribute) for attribute in attributes] if highlight and highlight(entry): - click.secho(template.format(symbol='*', *values), fg='green') + click.secho(template.format(symbol='*', *values), fg=COLORS['highlight']) else: click.secho(template.format(symbol=' ', *values)) From 09fb35b7194f9823948f07432f92411707a78901 Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Thu, 28 Nov 2019 13:07:59 +0100 Subject: [PATCH 2/2] Further improvements to `verdi computer test` output The number of tests is increased and messaging from within each test is disabled. The tests now return a tuple of a boolean indicating success or failure and an optional message. This message can be informational in case of success or an error message. With echo'ing only being performed in the main function, the output can be given a consistent look. Each test will have `[OK]` or `[FAILED]` on the same line with an optional message. Additional lines, such as for the full traceback, are prepended with newline and two spaces of indentation. --- aiida/cmdline/commands/cmd_computer.py | 163 +++++++++++++------------ 1 file changed, 88 insertions(+), 75 deletions(-) diff --git a/aiida/cmdline/commands/cmd_computer.py b/aiida/cmdline/commands/cmd_computer.py index 19271fc18d..b282228000 100644 --- a/aiida/cmdline/commands/cmd_computer.py +++ b/aiida/cmdline/commands/cmd_computer.py @@ -53,24 +53,19 @@ def prompt_for_computer_configuration(computer): # pylint: disable=unused-argum def _computer_test_get_jobs(transport, scheduler, authinfo): # pylint: disable=unused-argument - """ - Internal test to check if it is possible to check the queue state. + """Internal test to check if it is possible to check the queue state. :param transport: an open transport :param scheduler: the corresponding scheduler class :param authinfo: the AuthInfo object (from which one can get computer and aiidauser) - :return: True if the test succeeds, False if it fails. + :return: tuple of boolean indicating success or failure and an optional string message """ - echo.echo('> Getting job list...') found_jobs = scheduler.get_jobs(as_dict=True) - echo.echo(' [{} jobs found in the queue]'.format(len(found_jobs))) - click.secho(' [OK]', fg=echo.COLORS['success'], bold=echo.BOLD) - return True + return True, '{} jobs found in the queue'.format(len(found_jobs)) def _computer_test_no_unexpected_output(transport, scheduler, authinfo): # pylint: disable=unused-argument - """ - Test that there is no unexpected output from the connection. + """Test that there is no unexpected output from the connection. This can happen if e.g. there is some spurious command in the .bashrc or .bash_profile that is not guarded in case of non-interactive @@ -79,17 +74,16 @@ def _computer_test_no_unexpected_output(transport, scheduler, authinfo): # pyli :param transport: an open transport :param scheduler: the corresponding scheduler class :param authinfo: the AuthInfo object (from which one can get computer and aiidauser) - :return: True if the test succeeds, False if it fails. + :return: tuple of boolean indicating success or failure and an optional string message """ # Execute a command that should not return any error - echo.echo('> Checking that no spurious output is present...') retval, stdout, stderr = transport.exec_command_wait('echo -n') if retval != 0: - echo.echo_error("* ERROR! The command 'echo -n' returned a non-zero return code ({})!".format(retval)) - return False + return False, 'The command `echo -n` returned a non-zero return code ({})'.format(retval) + if stdout: - echo.echo_error( - u"""* ERROR! There is some spurious output in the standard output, + return False, u""" +There is some spurious output in the standard output, that we report below between the === signs: ========================================================= {} @@ -101,11 +95,10 @@ def _computer_test_no_unexpected_output(transport, scheduler, authinfo): # pyli https://github.com/aiidateam/aiida-core/issues/1890 (and in the AiiDA documentation, linked from that issue) """.format(stdout) - ) - return False + if stderr: - echo.echo_error( - u"""* ERROR! There is some spurious output in the stderr, + return u""" +There is some spurious output in the stderr, that we report below between the === signs: ========================================================= {} @@ -116,12 +109,21 @@ def _computer_test_no_unexpected_output(transport, scheduler, authinfo): # pyli shells, see comments in issue #1980 on GitHub: https://github.com/aiidateam/aiida-core/issues/1890 (and in the AiiDA documentation, linked from that issue) -""".format(stderr) - ) - return False +""" + + return True, None - click.secho(' [OK]', fg=echo.COLORS['success'], bold=echo.BOLD) - return True + +def _computer_get_remote_username(transport, scheduler, authinfo): # pylint: disable=unused-argument + """Internal test to check if it is possible to determine the username on the remote. + + :param transport: an open transport + :param scheduler: the corresponding scheduler class + :param authinfo: the AuthInfo object + :return: tuple of boolean indicating success or failure and an optional string message + """ + remote_user = transport.whoami() + return True, remote_user def _computer_create_temp_file(transport, scheduler, authinfo): # pylint: disable=unused-argument @@ -134,19 +136,14 @@ def _computer_create_temp_file(transport, scheduler, authinfo): # pylint: disab :param transport: an open transport :param scheduler: the corresponding scheduler class :param authinfo: the AuthInfo object (from which one can get computer and aiidauser) - :return: True if the test succeeds, False if it fails. + :return: tuple of boolean indicating success or failure and an optional string message """ import tempfile import datetime import os file_content = "Test from 'verdi computer test' on {}".format(datetime.datetime.now().isoformat()) - echo.echo('> Creating a temporary file in the work directory...') - echo.echo(' -> Getting the remote user name...') - remote_user = transport.whoami() - echo.echo(' [remote username: {}]'.format(remote_user)) - workdir = authinfo.get_workdir().format(username=remote_user) - echo.echo(' -> Checking/creating work directory: {}'.format(workdir)) + workdir = authinfo.get_workdir().format(username=transport.whoami()) try: transport.chdir(workdir) @@ -156,43 +153,34 @@ def _computer_create_temp_file(transport, scheduler, authinfo): # pylint: disab with tempfile.NamedTemporaryFile(mode='w+') as tempf: fname = os.path.split(tempf.name)[1] - echo.echo(' -> Creating the file {}...'.format(fname)) remote_file_path = os.path.join(workdir, fname) tempf.write(file_content) tempf.flush() transport.putfile(tempf.name, remote_file_path) - echo.echo(' -> Checking if the file has been created...') - if not transport.path_exists(remote_file_path): - echo.echo_error('* ERROR! The file was not found!') - return False - echo.echo(' [OK]') - echo.echo(' -> Retrieving the file and checking its content...') + if not transport.path_exists(remote_file_path): + return False, 'failed to create the file `{}` on the remote'.format(remote_file_path) handle, destfile = tempfile.mkstemp() os.close(handle) + try: transport.getfile(remote_file_path, destfile) with io.open(destfile, encoding='utf8') as dfile: read_string = dfile.read() - echo.echo(' [Retrieved]') + if read_string != file_content: - echo.echo_error('* ERROR! The file content is different from what was expected!') - echo.echo('** Expected:') - echo.echo(file_content) - echo.echo('** Found:') - echo.echo(read_string) - return False - - echo.echo(' [Content OK]') + message = 'retrieved file content is different from what was expected' + message += '\n Expected: {}'.format(file_content) + message += '\n Retrieved: {}'.format(read_string) + return False, message + finally: os.remove(destfile) - echo.echo(' -> Removing the file...') transport.remove(remote_file_path) - echo.echo(' [Deleted successfully]') - click.secho(' [OK]', fg=echo.COLORS['success'], bold=echo.BOLD) - return True + + return True, None def get_parameter_default(parameter, ctx): @@ -478,8 +466,8 @@ def computer_test(user, print_traceback, computer): to perform other tests. """ import traceback - from aiida.common.exceptions import NotExistent from aiida import orm + from aiida.common.exceptions import NotExistent # Set a user automatically if one is not specified in the command line if user is None: @@ -503,45 +491,70 @@ def computer_test(user, print_traceback, computer): num_failures = 0 num_tests = 0 + tests = { + _computer_test_no_unexpected_output: 'Checking for spurious output', + _computer_test_get_jobs: 'Getting number of jobs from scheduler', + _computer_get_remote_username: 'Determining remote user name', + _computer_create_temp_file: 'Creating and deleting temporary file' + } + try: - echo.echo('> Testing connection') + echo.echo('* Opening connection... ', nl=False) + with transport: - click.secho(' [OK]', fg=echo.COLORS['success'], bold=echo.BOLD) num_tests += 1 + echo.echo_highlight('[OK]', color='success') + scheduler.set_transport(transport) - for test in [_computer_test_no_unexpected_output, _computer_test_get_jobs, _computer_create_temp_file]: + + for test, test_label in tests.items(): + + echo.echo('* {}... '.format(test_label), nl=False) num_tests += 1 try: - succeeded = test(transport=transport, scheduler=scheduler, authinfo=authinfo) - # pylint:disable=broad-except - except Exception as error: - echo.echo_error('* The test raised an exception!') + success, message = test(transport=transport, scheduler=scheduler, authinfo=authinfo) + except Exception as exception: # pylint:disable=broad-except + success = False + message = '{}: {}'.format(exception.__class__.__name__, str(exception)) + if print_traceback: - echo.echo('** Full traceback:') - # Indent - echo.echo('\n'.join([' {}'.format(l) for l in traceback.format_exc().splitlines()])) + message += '\n Full traceback:\n' + message += '\n'.join([' {}'.format(l) for l in traceback.format_exc().splitlines()]) else: - echo.echo('** {}: {}'.format(error.__class__.__name__, error)) - echo.echo('** (use the --print-traceback option to see the full traceback)') - succeeded = False + message += '\n Use the `--print-traceback` option to see the full traceback.' - if not succeeded: + if not success: num_failures += 1 + if message: + echo.echo_highlight('[Failed]: ', color='error', nl=False) + echo.echo(message) + else: + echo.echo_highlight('[Failed]', color='error') + else: + if message: + echo.echo_highlight('[OK]: ', color='success', nl=False) + echo.echo(message) + else: + echo.echo_highlight('[OK]', color='success') if num_failures: - echo.echo_warning('Some tests failed! ({} out of {} failed)'.format(num_failures, num_tests)) + echo.echo_warning('{} out of {} tests failed'.format(num_failures, num_tests)) else: - echo.echo_success('All {} tests succeeded)'.format(num_tests)) - except Exception as error: # pylint:disable=broad-except - echo.echo_error('** Error while trying to connect to the computer! Cannot perform following tests, stopping.') + echo.echo_success('all {} tests succeeded'.format(num_tests)) + + except Exception as exception: # pylint:disable=broad-except + echo.echo_highlight('[FAILED]: ', color='error', nl=False) + message = 'Error while trying to connect to the computer' + if print_traceback: - echo.echo('** Full traceback:') - echo.echo('\n'.join([' {}'.format(l) for l in traceback.format_exc().splitlines()])) + message += '\n Full traceback:\n' + message += '\n'.join([' {}'.format(l) for l in traceback.format_exc().splitlines()]) else: - echo.echo('{}: {}'.format(error.__class__.__name__, error)) - echo.echo('(use the --print-traceback option to see the full traceback)') - succeeded = False + message += '\n Use the `--print-traceback` option to see the full traceback.' + + echo.echo(message) + echo.echo_warning('{} out of {} tests failed'.format(1, num_tests)) @verdi_computer.command('delete')