From c85e929c2227c9882868c969c76573a7fa092d2e Mon Sep 17 00:00:00 2001
From: Sebastiaan Huber <mail@sphuber.net>
Date: Thu, 20 Aug 2020 22:21:00 +0200
Subject: [PATCH] `verdi computer test`: fix bug in spurious output test

The test that checks for spurious output when executing a normal command
on a computer over the transport had a bug in it that went unnoticed
because the code path was not tested. If the `stderr` of the command
contained any output the command would raise because the test that is
called `_computer_test_no_unexpected_output` would incorrectly return a
tuple of length one instead of two in that case.

In addition to adding tests to hit this code path, the message that is
printed in the case of non-empty stdout or stderr is deduplicated and
adapted to be bit clearer and refer directly to the documentation
instead of through a Github issue.
---
 aiida/cmdline/commands/cmd_computer.py  | 37 ++++++++-----------------
 tests/cmdline/commands/test_computer.py | 36 ++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/aiida/cmdline/commands/cmd_computer.py b/aiida/cmdline/commands/cmd_computer.py
index 072dd07585..72b81d3b1d 100644
--- a/aiida/cmdline/commands/cmd_computer.py
+++ b/aiida/cmdline/commands/cmd_computer.py
@@ -76,35 +76,20 @@ def _computer_test_no_unexpected_output(transport, scheduler, authinfo):  # pyli
     if retval != 0:
         return False, 'The command `echo -n` returned a non-zero return code ({})'.format(retval)
 
-    if stdout:
-        return False, u"""
-There is some spurious output in the standard output,
-that we report below between the === signs:
-=========================================================
+    template = """
+We detected some spurious output in the {} when connecting to the computer, as shown between the bars
+=====================================================================================================
 {}
-=========================================================
-Please check that you don't have code producing output in
-your ~/.bash_profile (or ~/.bashrc). If you don't want to
-remove the code, but just to disable it for non-interactive
-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(stdout)
+=====================================================================================================
+Please check that you don't have code producing output in your ~/.bash_profile, ~/.bashrc or similar.
+If you don't want to remove the code, but just to disable it for non-interactive shells, see comments
+in this troubleshooting section of the online documentation: https://bit.ly/2FCRDc5
+"""
+    if stdout:
+        return False, template.format('stdout', stdout)
 
     if stderr:
-        return u"""
-There is some spurious output in the stderr,
-that we report below between the === signs:
-=========================================================
-{}
-=========================================================
-Please check that you don't have code producing output in
-your ~/.bash_profile (or ~/.bashrc). If you don't want to
-remove the code, but just to disable it for non-interactive
-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)
-"""
+        return False, template.format('stderr', stderr)
 
     return True, None
 
diff --git a/tests/cmdline/commands/test_computer.py b/tests/cmdline/commands/test_computer.py
index eddad9dae4..4ebea32d46 100644
--- a/tests/cmdline/commands/test_computer.py
+++ b/tests/cmdline/commands/test_computer.py
@@ -753,3 +753,39 @@ def test_interactive(clear_database_before_test, aiida_localhost, non_interactiv
     # For now I'm not writing anything in them
     assert new_computer.get_prepend_text() == ''
     assert new_computer.get_append_text() == ''
+
+
+@pytest.mark.usefixtures('clear_database_before_test')
+def test_computer_test_stderr(run_cli_command, aiida_localhost, monkeypatch):
+    """Test `verdi computer test` where tested command returns non-empty stderr."""
+    from aiida.transports.plugins.local import LocalTransport
+
+    aiida_localhost.configure()
+    stderr = 'spurious output in standard error'
+
+    def exec_command_wait(self, command, **kwargs):
+        return 0, '', stderr
+
+    monkeypatch.setattr(LocalTransport, 'exec_command_wait', exec_command_wait)
+
+    result = run_cli_command(computer_test, [aiida_localhost.label])
+    assert 'Warning: 1 out of 5 tests failed' in result.output
+    assert stderr in result.output
+
+
+@pytest.mark.usefixtures('clear_database_before_test')
+def test_computer_test_stdout(run_cli_command, aiida_localhost, monkeypatch):
+    """Test `verdi computer test` where tested command returns non-empty stdout."""
+    from aiida.transports.plugins.local import LocalTransport
+
+    aiida_localhost.configure()
+    stdout = 'spurious output in standard output'
+
+    def exec_command_wait(self, command, **kwargs):
+        return 0, stdout, ''
+
+    monkeypatch.setattr(LocalTransport, 'exec_command_wait', exec_command_wait)
+
+    result = run_cli_command(computer_test, [aiida_localhost.label])
+    assert 'Warning: 1 out of 5 tests failed' in result.output
+    assert stdout in result.output