[cli] Disable spinner animation on non-live TTYs

src/client/cli/cmd/animated_spinner.cpp

@@ -16,6 +16,7 @@
 
 #include "animated_spinner.h"
 
+#include <cassert>
 #include <iostream>
 
 namespace mp = multipass;
@@ -29,8 +30,8 @@ void clear_line(std::ostream& out)
 }
 } // namespace
 
-mp::AnimatedSpinner::AnimatedSpinner(std::ostream& cout)
-    : spinner{'|', '/', '-', '\\'}, cout{cout}, running{false}
+mp::AnimatedSpinner::AnimatedSpinner(std::ostream& cout, bool is_live)
+    : spinner{'|', '/', '-', '\\'}, cout{cout}, running{false}, is_live(is_live)
 {
 }
 
@@ -42,19 +43,26 @@ mp::AnimatedSpinner::~AnimatedSpinner()
 void mp::AnimatedSpinner::start(const std::string& start_message)
 {
     std::unique_lock<decltype(mutex)> lock{mutex};
+
     if (!running)
     {
         current_message = start_message;
         running = true;
-        clear_line(cout);
-        cout << start_message << "  " << std::flush;
-        t = std::thread(&AnimatedSpinner::draw, this);
+
+        if (is_live)
+        {
+            clear_line(cout);
+            cout << start_message << "  " << std::flush;
+            t = std::thread(&AnimatedSpinner::draw, this);
+        }
+        else
+            cout << start_message << std::endl;
     }
 }
 
 void mp::AnimatedSpinner::start()
 {
-    if (!current_message.empty())
+    if (is_live && !current_message.empty())
         start(current_message);
 }
 
@@ -64,12 +72,17 @@ void mp::AnimatedSpinner::stop()
     if (running)
     {
         running = false;
-        cv.notify_one();
-        lock.unlock();
-        if (t.joinable())
-            t.join();
+        if (is_live)
+        {
+            cv.notify_one();
+            lock.unlock();
+            if (t.joinable())
+                t.join();
+        }
     }
-    clear_line(cout);
+
+    if (is_live)
+        clear_line(cout);
 }
 
 void mp::AnimatedSpinner::print(std::ostream& stream, const std::string& message)
@@ -84,6 +97,9 @@ void mp::AnimatedSpinner::print(std::ostream& stream, const std::string& message
 void mp::AnimatedSpinner::draw()
 {
     std::unique_lock<decltype(mutex)> lock{mutex};
+
+    assert(is_live && "should only draw when running live");
+
     auto it = spinner.begin();
     while (running)
     {
@@ -92,6 +108,5 @@ void mp::AnimatedSpinner::draw()
         cout << "\b" << *it << std::flush;
         cv.wait_for(lock, std::chrono::milliseconds(100));
     }
-    cout << "\b"
-         << " " << std::flush;
+    cout << "\b" << " " << std::flush;
 }

src/client/cli/cmd/animated_spinner.h

@@ -27,7 +27,7 @@ namespace multipass
 class AnimatedSpinner
 {
 public:
-    explicit AnimatedSpinner(std::ostream& cout);
+    explicit AnimatedSpinner(std::ostream& cout, bool is_live = true);
     ~AnimatedSpinner();
 
     void start(const std::string& message);
@@ -44,5 +44,6 @@ class AnimatedSpinner
     std::mutex mutex;
     std::condition_variable cv;
     std::thread t;
+    const bool is_live;
 };
 } // namespace multipass

src/client/cli/cmd/clone.cpp

@@ -33,7 +33,7 @@ mp::ReturnCodeVariant cmd::Clone::run(ArgParser* parser)
         return parser->returnCodeFrom(parscode);
     }
 
-    AnimatedSpinner spinner{cout};
+    AnimatedSpinner spinner{cout, term->cout_is_live()};
     auto action_on_success = [this, &spinner](CloneReply& reply) -> ReturnCodeVariant {
         spinner.stop();
         cout << reply.reply_message();

src/client/cli/cmd/launch.cpp

@@ -468,7 +468,7 @@ mp::ReturnCodeVariant cmd::Launch::request_launch(const ArgParser* parser)
 {
     if (!spinner)
         spinner = std::make_unique<multipass::AnimatedSpinner>(
-            cout); // Creating just in time to work around canonical/multipass#2075
+            cout, term->cout_is_live()); // Creating just in time to work around canonical/multipass#2075
 
     if (timer)
         timer->resume();

src/client/cli/cmd/mount.cpp

@@ -78,7 +78,7 @@ mp::ReturnCodeVariant cmd::Mount::run(mp::ArgParser* parser)
         return parser->returnCodeFrom(ret);
     }
 
-    mp::AnimatedSpinner spinner{cout};
+    mp::AnimatedSpinner spinner{cout, term->cout_is_live()};
 
     auto on_success = [&spinner](mp::MountReply& reply) -> ReturnCodeVariant {
         spinner.stop();

src/client/cli/cmd/restart.cpp

@@ -38,7 +38,7 @@ mp::ReturnCodeVariant cmd::Restart::run(mp::ArgParser* parser)
     if (ret != ParseCode::Ok)
         return parser->returnCodeFrom(ret);
 
-    AnimatedSpinner spinner{cout};
+    AnimatedSpinner spinner{cout, term->cout_is_live()};
     auto on_success = [this, &spinner](mp::RestartReply& reply) -> ReturnCodeVariant {
         spinner.stop();
         if (term->is_live() && update_available(reply.update_info()))

src/client/cli/cmd/restore.cpp

@@ -31,7 +31,7 @@ mp::ReturnCodeVariant cmd::Restore::run(mp::ArgParser* parser)
     if (auto ret = parse_args(parser); ret != ParseCode::Ok)
         return parser->returnCodeFrom(ret);
 
-    AnimatedSpinner spinner{cout};
+    AnimatedSpinner spinner{cout, term->cout_is_live()};
 
     auto on_success = [this, &spinner](mp::RestoreReply& reply) -> ReturnCodeVariant {
         spinner.stop();

src/client/cli/cmd/snapshot.cpp

@@ -31,7 +31,7 @@ mp::ReturnCodeVariant cmd::Snapshot::run(mp::ArgParser* parser)
     if (auto ret = parse_args(parser); ret != ParseCode::Ok)
         return parser->returnCodeFrom(ret);
 
-    AnimatedSpinner spinner{cout};
+    AnimatedSpinner spinner{cout, term->cout_is_live()};
 
     auto on_success = [this, &spinner](mp::SnapshotReply& reply) -> ReturnCodeVariant {
         spinner.stop();

src/client/cli/cmd/start.cpp

@@ -55,7 +55,7 @@ mp::ReturnCodeVariant cmd::Start::run(mp::ArgParser* parser)
         return parser->returnCodeFrom(ret);
     }
 
-    AnimatedSpinner spinner{cout};
+    AnimatedSpinner spinner{cout, term->cout_is_live()};
 
     auto on_success = [&spinner, this](mp::StartReply& reply) -> ReturnCodeVariant {
         spinner.stop();

src/client/cli/cmd/stop.cpp

@@ -39,7 +39,7 @@ mp::ReturnCodeVariant cmd::Stop::run(mp::ArgParser* parser)
 
     auto on_success = [](mp::StopReply& reply) -> ReturnCodeVariant { return ReturnCode::Ok; };
 
-    AnimatedSpinner spinner{cout};
+    AnimatedSpinner spinner{cout, term->cout_is_live()};
     auto on_failure = [this, &spinner](grpc::Status& status) -> ReturnCodeVariant {
         spinner.stop();
 

src/client/cli/cmd/suspend.cpp

@@ -38,7 +38,7 @@ mp::ReturnCodeVariant cmd::Suspend::run(mp::ArgParser* parser)
 
     auto on_success = [](mp::SuspendReply& reply) -> ReturnCodeVariant { return ReturnCode::Ok; };
 
-    AnimatedSpinner spinner{cout};
+    AnimatedSpinner spinner{cout, term->cout_is_live()};
     auto on_failure = [this, &spinner](grpc::Status& status) -> ReturnCodeVariant {
         spinner.stop();
         return standard_failure_handler_for(name(), cerr, status);

src/client/cli/cmd/wait_ready.cpp

@@ -36,7 +36,7 @@ mp::ReturnCodeVariant cmd::WaitReady::run(mp::ArgParser* parser)
         return parser->returnCodeFrom(ret);
     }
 
-    mp::AnimatedSpinner spinner{cout};
+    mp::AnimatedSpinner spinner{cout, term->cout_is_live()};
     spinner.start("Waiting for the Multipass daemon to be ready");
 
     std::unique_ptr<mp::utils::Timer> timer;

tests/unit/test_common_callbacks.cpp

@@ -17,6 +17,7 @@
 #include "common.h"
 #include "mock_client_platform.h"
 #include "mock_client_rpc.h"
+#include "multipass/utils.h"
 #include "stub_terminal.h"
 
 #include <src/client/cli/cmd/animated_spinner.h>
@@ -26,6 +27,7 @@
 #include <sstream>
 
 namespace mp = multipass;
+namespace mpu = mp::utils;
 namespace mpt = mp::test;
 using namespace testing;
 
@@ -199,3 +201,55 @@ TEST_F(TestSpinnerCallbacks, confirmationCallbackAnswers)
     EXPECT_THAT(err.str(), IsEmpty());
     EXPECT_THAT(out.str(), clearStreamMatcher());
 }
+
+TEST_F(TestSpinnerCallbacks, nonLiveLoggingSpinnerCallbackLogsWithoutSpinnerArtifacts)
+{
+    constexpr auto log = "message in a bottle";
+    mp::AnimatedSpinner non_live_spinner{out, false};
+
+    mp::MountReply reply;
+    reply.set_log_line(log);
+
+    auto cb =
+        mp::make_logging_spinner_callback<mp::MountRequest, mp::MountReply>(non_live_spinner, err);
+    cb(reply, nullptr);
+
+    EXPECT_THAT(err.str(), StrEq(log));
+    EXPECT_THAT(out.str(), IsEmpty());
+}
+
+TEST_F(TestSpinnerCallbacks, nonLiveReplySpinnerCallbackPrintsAllMessagesWithoutSpinnerArtifacts)
+{
+    constexpr auto log = "message in a bottle";
+    constexpr auto msg = "answer";
+    mp::AnimatedSpinner non_live_spinner{out, false};
+
+    mp::MountReply reply;
+    reply.set_log_line(log);
+    reply.set_reply_message(msg);
+
+    auto cb =
+        mp::make_reply_spinner_callback<mp::MountRequest, mp::MountReply>(non_live_spinner, err);
+    cb(reply, nullptr);
+
+    EXPECT_THAT(err.str(), StrEq(log));
+    EXPECT_THAT(mpu::trim_end(out.str()), StrEq(msg));
+}
+
+TEST_F(TestSpinnerCallbacks, nonLiveIterativeSpinnerCallbackPrintsAllMessagesWithoutSpinnerArtifacts)
+{
+    constexpr auto log = "message in a bottle";
+    constexpr auto msg = "answer";
+    mp::AnimatedSpinner non_live_spinner{out, false};
+
+    mp::MountReply reply;
+    reply.set_log_line(log);
+    reply.set_reply_message(msg);
+
+    auto cb = mp::make_iterative_spinner_callback<mp::MountRequest, mp::MountReply>(
+        non_live_spinner, term);
+    cb(reply, nullptr);
+
+    EXPECT_THAT(err.str(), StrEq(log));
+    EXPECT_THAT(mpu::trim_end(out.str()), StrEq(msg));
+}