Code review comment for lp:~nick-dedekind/mir/1355173.trust-prompt-suspend

> Needs fixing:
>
> Missing acceptance tests - I suppose something similar to:
> "TEST_F(PromptSessionClientAPI, notifies_start_and_stop)"
>
>
> ===
> 733 + // Need to verify explicitly as we see unmatched callbacks during
> teardown of fixture
> 734 + Mock::VerifyAndClearExpectations(&prompt_session_listener);
> ===
> Not needed.
>
> ===
> 716 // Need to verify explicitly as we see unmatched callbacks during
> teardown of fixture
> 717 Mock::VerifyAndClearExpectations(&prompt_session_listener);
> ===
> Pre-existing, but not needed either.
>
> ===
> 198 + std::unique_lock<std::mutex> lk(guard);
> 210 + std::unique_lock<std::mutex> lk(guard);
> 222 + std::unique_lock<std::mutex> lk(guard);
> 234 + std::unique_lock<std::mutex> lk(guard);
> ===
> Prefer std::lock_guard<std::mutex> as no features of unique_lock are used.
>
> ===
> 605 +TEST_F(PromptSession, start_prompt_session_when_stopped)
> 616 +TEST_F(PromptSession, stop_prompt_session_when_started)
> 628 +TEST_F(PromptSession, suspend_prompt_session_when_started)
> 640 +TEST_F(PromptSession,
> suspend_prompt_session_fails_to_stop_helper_when_not_started)
> 650 +TEST_F(PromptSession, resume_prompt_session_when_suspended)
> 663 +TEST_F(PromptSession,
> resume_prompt_session_fails_to_stop_helper_when_not_started)
> ===
> Redundant prompt_session in test case name
> Perhaps something like:
>
> TEST_F(PromptSession, can_start)
> TEST_F(PromptSession, can_stop)
> TEST_F(PromptSession, can_suspend)
> TEST_F(PromptSession, fails_to_suspend_if_not_in_started_state)
> TEST_F(PromptSession, can_resume)
> TEST_F(PromptSession, fails_to_resume_if_in_stopped_state)

Done.

>
> ===
> 204 + helper_session->start_prompt_session();
> ===
> e. al. Do you need to keep the mutex locked while making these calls?
> Preferably, unlock the mutex before making them (in which case disregard the
> suggestion for std::lock_guard above).

If we don't then we can end up interleaving calls to start/stop/suspend.