assistant: add integration test for turn up volume

Bug: 149759085
Test: run integration tests
Change-Id: Ib6ef5db411f7e1eb6571ca5568178536e726b1a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062685
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742783}

chrome/browser/ui/ash/assistant/assistant_browser_tests.md

@@ -5,14 +5,16 @@
 This documentation concerns all Assistant browser/integration test which use
 `AssistantTestMixin` or any derived class.
 
-All these tests run the real LibAssistant code, and use a [FakeS3Server](go/fake-s3-for-libassistant) to
-intercept and replay the communication between LibAssistant and the cloud.
-This allows these tests to run in the CQ, without requiring any network connectivity.
+All these tests run the real LibAssistant code, and use a
+[FakeS3Server](go/fake-s3-for-libassistant) to intercept and replay the
+communication between LibAssistant and the cloud.  This allows these tests to
+run in the CQ, without requiring any network connectivity.
 
 ## Adding a new test
 
-To add a new integration test, your test fixture must inherit from `MixinBasedInProcessBrowserTest`
-and must contain `AssistantTestMixin` as a private variable:
+To add a new integration test, your test fixture must inherit from
+`MixinBasedInProcessBrowserTest` and must contain `AssistantTestMixin` as a
+private variable:
 
     #include "chrome/browser/ui/ash/assistant/assistant_test_mixin.h"
     #include "chrome/test/base/mixin_based_in_process_browser_test.h"
@@ -25,14 +27,18 @@ and must contain `AssistantTestMixin` as a private variable:
       AssistantTestMixin* tester() { return &tester_; }
 
      private:
-      AssistantTestMixin tester_{&mixin_host_, this, embedded_test_server(), FakeS3Mode::kReplay};
+      AssistantTestMixin tester_{&mixin_host_, this,
+                                 embedded_test_server(),
+                                 FakeS3Mode::kReplay};
 
     };
 
-Tests can use API provided by `AssistantTextMixin` to interact with the Assistant UI.
-Look in `chrome/browser/ui/ash/assistant/assistant_test_mixin.h` to see all the available methods.
+Tests can use API provided by `AssistantTextMixin` to interact with the
+Assistant UI.  Look in `chrome/browser/ui/ash/assistant/assistant_test_mixin.h`
+to see all the available methods.
 
-Each test *must* start by starting the Assistant service and waiting for it to signal it's ready.
+Each test *must* start by starting the Assistant service and waiting for it to
+signal it's ready.
 That is done by calling `tester()->StartAssistantAndWaitForReady()`.
 
     IN_PROC_BROWSER_TEST_F(MyBrowserTest, MyVeryFirstTest) {
@@ -43,37 +49,40 @@ That is done by calling `tester()->StartAssistantAndWaitForReady()`.
 
 ## Using the fake S3 server
 
-The fake S3 server can run in 3 different modes,
-all of which you'll need to use while adding new tests.
+The fake S3 server can run in 3 different modes, all of which you'll need to
+use while adding new tests.
 
-You'll start by using the [_proxy_ mode](#in-proxy-mode) while developing a new test; This simply forwards all
-requests to a real S3 server.
+You'll start by using the [_proxy_ mode](#in-proxy-mode) while developing a new
+test; This simply forwards all requests to a real S3 server.
 
-When you're ready to commit you'll switch to [_record_ mode](#in-record-mode) in order to record the interaction
-between LibAssistant and the S3 server.
+When you're ready to commit you'll switch to [_record_ mode](#in-record-mode)
+in order to record the interaction between LibAssistant and the S3 server.
 
-Afterwards you'll switch to [_replay_ mode](#in-replay-mode); In this mode the fake S3 server will replay the
-recorded interactions. This allows the CQ to run the tests without once requiring network
-connectivity to contact the real S3 server.
+Afterwards you'll switch to [_replay_ mode](#in-replay-mode); In this mode the
+fake S3 server will replay the recorded interactions. This allows the CQ to run
+the tests without once requiring network connectivity to contact the real S3
+server.
 
 ## Running the tests
 
 ### In proxy mode
 
-In this mode the fake S3 server works as a simple proxy, forwarding all LibAssistant
-requests to the real S3 server (and proxying the responses back).
+In this mode the fake S3 server works as a simple proxy, forwarding all
+LibAssistant requests to the real S3 server (and proxying the responses back).
 
 As such, you will have to:
 
 1. Tell the fake S3 server to use this mode.
 2. Generate a real Gaia authentication token for the communication with the real S3 server.
 
-The first part can be achieved by passing `|FakeS3Mode::kProxy|` as the final argument
-to the constructor of the `|AssistantTextMixin|` instance in your test fixture.
+The first part can be achieved by passing `|FakeS3Mode::kProxy|` as the final
+argument to the constructor of the `|AssistantTextMixin|` instance in your test
+fixture.
 
     // in your test fixture
     private:
-      AssistantTestMixin tester_{&mixin_host_, this, embedded_test_server(), FakeS3Mode::kProxy};
+      AssistantTestMixin tester_{&mixin_host_, this, embedded_test_server(),
+                                 FakeS3Mode::kProxy};
 
 The second part is achieved by running your tests through the wrapper script
 `chromeos/assistant/internal/test_support/run_with_access_token.py`.
@@ -92,16 +101,18 @@ Alternatively you can also store the Gaia token you'd like to use in the environ
 
 ### In record mode
 
-In this mode the fake S3 server will again work as a proxy, but it will also store
-a recording of all the communications in a file called
+In this mode the fake S3 server will again work as a proxy, but it will also
+store a recording of all the communications in a file called
 
     chromeos/assistant/internal/test_data/<testfixture_testname>.fake_s3.proto
 
 A separate file is created for each test.
 
-You will need to run once in *record* mode when your test is ready to be committed.
+You will need to run once in *record* mode when your test is ready to be
+committed.
 
-To do this, use the same steps described [above](#in-proxy-mode) but replace `|FakeS3Mode:kProxy|` with `|FakeS3Mode::kRecord|`.
+To do this, use the same steps described [above](#in-proxy-mode) but replace
+`|FakeS3Mode:kProxy|` with `|FakeS3Mode::kRecord|`.
 
 Note that these `.fake_s3.proto` files will need to be committed before you can
 commit your changes. See [here](#committing-your-tests) for more information.
@@ -119,7 +130,8 @@ to the constructor of the `|AssistantTextMixin|` instance in your test fixture.
 
     // in your test fixture
     private:
-      AssistantTestMixin tester_{&mixin_host_, this, embedded_test_server(), FakeS3Mode::kReplay};
+      AssistantTestMixin tester_{&mixin_host_, this, embedded_test_server(),
+                                 FakeS3Mode::kReplay};
 
 This is the mode the CQ runs in, and as such you must **make sure you've enabled this mode before pushing your CL!**
 
@@ -138,7 +150,8 @@ file and use [_record_ mode](#in-record-mode) to generate a new proto file.
 
 Committing the tests has to be done in multiple steps:
 
-1. Create an internal CL to add the new files in `chromeos/assistant/internal/test_data`.
+1. Create an internal CL to add the new files in
+   `chromeos/assistant/internal/test_data`.
 2. Uprev this CL.
 3. Create a CL for your browsertest changes
 4. (Optionally): If you deleted tests, create an internal CL to remove their `.fake_s3.proto` files from `chromeos/assistant/internal/test_data`.

chrome/browser/ui/ash/assistant/assistant_browsertest.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/ash/assistant/assistant_test_mixin.h"
 #include "chrome/test/base/mixin_based_in_process_browser_test.h"
+#include "chromeos/audio/cras_audio_handler.h"
 
 namespace chromeos {
 namespace assistant {
@@ -62,5 +63,27 @@ IN_PROC_BROWSER_TEST_F(AssistantBrowserTest, ShouldDisplayCardResponse) {
   tester()->ExpectCardResponse("Mount Everest");
 }
 
+IN_PROC_BROWSER_TEST_F(AssistantBrowserTest, ShouldTurnUpVolume) {
+  tester()->StartAssistantAndWaitForReady();
+
+  ShowAssistantUi();
+
+  EXPECT_TRUE(tester()->IsVisible());
+
+  auto* cras = chromeos::CrasAudioHandler::Get();
+  constexpr int kStartVolumePercent = 50;
+  cras->SetOutputVolumePercent(kStartVolumePercent);
+  EXPECT_EQ(kStartVolumePercent, cras->GetOutputVolumePercent());
+
+  tester()->SendTextQuery("turn up volume");
+
+  tester()->ExpectResult(true, base::BindRepeating(
+                                   [](chromeos::CrasAudioHandler* cras) {
+                                     return cras->GetOutputVolumePercent() >
+                                            kStartVolumePercent;
+                                   },
+                                   cras));
+}
+
 }  // namespace assistant
 }  // namespace chromeos

chrome/browser/ui/ash/assistant/assistant_test_mixin.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/ash/assistant/assistant_test_mixin.h"
 
+#include <utility>
+
 #include "ash/assistant/model/ui/assistant_card_element.h"
 #include "ash/assistant/ui/assistant_ui_constants.h"
 #include "ash/assistant/ui/main_stage/assistant_ui_element_view.h"
@@ -214,6 +216,23 @@ class CardResponseWaiter : public ResponseWaiter {
   }
 };
 
+template <typename T>
+void CheckResult(base::OnceClosure quit,
+                 T expected_value,
+                 base::RepeatingCallback<T()> value_callback) {
+  if (expected_value == value_callback.Run()) {
+    std::move(quit).Run();
+    return;
+  }
+
+  // Check again in the future
+  base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(CheckResult<T>, std::move(quit), expected_value,
+                     value_callback),
+      base::TimeDelta::FromMilliseconds(10));
+}
+
 }  // namespace
 
 // Test mixin for the browser tests that logs in the given user and issues
@@ -351,6 +370,27 @@ void AssistantTestMixin::SendTextQuery(const std::string& query) {
   test_api_->SendTextQuery(query);
 }
 
+template <typename T>
+void AssistantTestMixin::ExpectResult(
+    T expected_value,
+    base::RepeatingCallback<T()> value_callback) {
+  const base::test::ScopedRunLoopTimeout run_timeout(FROM_HERE,
+                                                     kDefaultWaitTimeout);
+
+  // Wait until we're ready or we hit the timeout.
+  base::RunLoop run_loop;
+  CheckResult(run_loop.QuitClosure(), expected_value, value_callback);
+
+  EXPECT_NO_FATAL_FAILURE(run_loop.Run())
+      << "Failed waiting for expected result.\n"
+      << "Expected \"" << expected_value << "\"\n"
+      << "Got \"" << value_callback.Run() << "\"";
+}
+
+template void AssistantTestMixin::ExpectResult<bool>(
+    bool expected_value,
+    base::RepeatingCallback<bool()> value_callback);
+
 void AssistantTestMixin::ExpectCardResponse(
     const std::string& expected_response,
     base::TimeDelta wait_timeout) {

chrome/browser/ui/ash/assistant/assistant_test_mixin.h

@@ -7,6 +7,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "base/macros.h"
 #include "base/time/time.h"
@@ -67,6 +68,17 @@ class AssistantTestMixin : public InProcessBrowserTestMixin {
   // displaying the query input text field.
   void SendTextQuery(const std::string& query);
 
+  // Check if the |expected_value| is equal to the result of running
+  // |value_callback|.  This method will block and continuously try the
+  // comparison above until it succeeds, or timeout.
+  //
+  // NOTE: This is a template method. If you need to use it with a new type,
+  // you may see a link error. You will need to manually instantiate for the
+  // new type.  Please see .cc file for examples.
+  template <typename T>
+  void ExpectResult(T expected_value,
+                    base::RepeatingCallback<T()> value_callback);
+
   // Waits until a card response is rendered that contains the given text.
   // If |expected_response| is not received in |wait_timeout|, this will fail
   // the test.