Skip to content

Commit

Permalink
Added TestFuture to web_kiosk_app_update_observer_unittest
Browse files Browse the repository at this point in the history 
Replacing the usage of base::RunLoop with base::test::TestFuture for better code quality in /chrome/browser/ash/app_mode/.

Bug: b/255714505
Test: Ran test involved in the change.
Change-Id: If6882f940746813217940e9735b5d4a77b2d1c02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3991210
Reviewed-by: Ben Franz <bfranz@chromium.org>
Reviewed-by: Jeroen Dhollander <jeroendh@google.com>
Commit-Queue: Ashutosh Singhal <macinashutosh@google.com>
Cr-Commit-Position: refs/heads/main@{#1067476}
  • Loading branch information
Ashutosh Singhal authored and Chromium LUCI CQ committed Nov 4, 2022
1 parent edc5dea commit ab7843c
Showing 1 changed file with 86 additions and 73 deletions.
159 changes: 86 additions & 73 deletions chrome/browser/ash/app_mode/web_app/web_kiosk_app_update_observer_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
#include <memory>
#include <vector>

#include "base/run_loop.h"
#include "base/scoped_observation.h"
#include "base/test/bind.h"
#include "base/test/gmock_callback_support.h"
#include "base/test/test_future.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
#include "chrome/browser/apps/app_service/app_service_test.h"
Expand All @@ -39,7 +39,18 @@ namespace ash {

namespace {

using base::test::RunClosure;
#define EXEC_AND_WAIT_FOR_CALL(exec, mock, method) \
({ \
TestFuture<bool> waiter; \
EXPECT_CALL(mock, method).WillOnce(Invoke([&]() { \
waiter.SetValue(true); \
})); \
exec; \
EXPECT_TRUE(waiter.Wait()); \
})

using base::test::TestFuture;
using testing::Invoke;

const char kAppId[] = "testappid";
const char kAppEmail[] = "test@example.com";
Expand Down Expand Up @@ -203,14 +214,14 @@ TEST_F(WebKioskAppUpdateObserverTest, ShouldUpdateAppInfoWithIconWhenReady) {

// Initial app info without icon.
{
base::RunLoop loop;
EXPECT_CALL(*mock_app_registry_observer(), OnAppUpdate)
.WillOnce(RunClosure(loop.QuitClosure()));
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
loop.Run();
EXEC_AND_WAIT_FOR_CALL(
{
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
},
*mock_app_registry_observer(), OnAppUpdate);
}

EXPECT_EQ(app_data()->status(), WebKioskAppData::Status::kInstalled);
Expand All @@ -220,30 +231,30 @@ TEST_F(WebKioskAppUpdateObserverTest, ShouldUpdateAppInfoWithIconWhenReady) {

// Update app info.
{
base::RunLoop loop;
EXPECT_CALL(*mock_app_registry_observer(), OnAppUpdate)
.WillOnce(RunClosure(loop.QuitClosure()));
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->name = kAppTitle2;
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
loop.Run();
EXEC_AND_WAIT_FOR_CALL(
{
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->name = kAppTitle2;
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
},
*mock_app_registry_observer(), OnAppUpdate);
}

EXPECT_EQ(app_data()->name(), kAppTitle2);

// Update app icon.
{
base::RunLoop loop;
EXPECT_CALL(*mock_app_registry_observer(), OnAppUpdate)
.WillOnce(RunClosure(loop.QuitClosure()));
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->icon_key = apps::IconKey();
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
loop.Run();
EXEC_AND_WAIT_FOR_CALL(
{
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->icon_key = apps::IconKey();
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
},
*mock_app_registry_observer(), OnAppUpdate);
}

EXPECT_FALSE(app_data()->icon().isNull());
Expand All @@ -258,15 +269,15 @@ TEST_F(WebKioskAppUpdateObserverTest, ShouldNotUpdateAppInfoWhenNotReady) {
EXPECT_NE(app_data()->launch_url(), kAppLaunchUrl);

{
base::RunLoop loop;
EXPECT_CALL(*mock_app_registry_observer(), OnAppUpdate)
.WillOnce(RunClosure(loop.QuitClosure()));
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->readiness = apps::Readiness::kUnknown;
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
loop.Run();
EXEC_AND_WAIT_FOR_CALL(
{
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->readiness = apps::Readiness::kUnknown;
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
},
*mock_app_registry_observer(), OnAppUpdate);
}

EXPECT_EQ(app_data()->status(), WebKioskAppData::Status::kInit);
Expand All @@ -281,15 +292,15 @@ TEST_F(WebKioskAppUpdateObserverTest, ShouldNotUpdateAppInfoForNonWebApps) {
EXPECT_NE(app_data()->launch_url(), kAppLaunchUrl);

{
base::RunLoop loop;
EXPECT_CALL(*mock_app_registry_observer(), OnAppUpdate)
.WillOnce(RunClosure(loop.QuitClosure()));
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->app_type = apps::AppType::kChromeApp;
app_service()->OnApps(std::move(apps), apps::AppType::kChromeApp,
/*should_notify_initialized=*/true);
loop.Run();
EXEC_AND_WAIT_FOR_CALL(
{
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->app_type = apps::AppType::kChromeApp;
app_service()->OnApps(std::move(apps), apps::AppType::kChromeApp,
/*should_notify_initialized=*/true);
},
*mock_app_registry_observer(), OnAppUpdate);
}

EXPECT_EQ(app_data()->status(), WebKioskAppData::Status::kInit);
Expand All @@ -304,15 +315,15 @@ TEST_F(WebKioskAppUpdateObserverTest, ShouldNotUpdateAppInfoForNonKioskApps) {
EXPECT_NE(app_data()->launch_url(), kAppLaunchUrl);

{
base::RunLoop loop;
EXPECT_CALL(*mock_app_registry_observer(), OnAppUpdate)
.WillOnce(RunClosure(loop.QuitClosure()));
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->install_reason = apps::InstallReason::kPolicy;
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
loop.Run();
EXEC_AND_WAIT_FOR_CALL(
{
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->install_reason = apps::InstallReason::kPolicy;
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
},
*mock_app_registry_observer(), OnAppUpdate);
}

EXPECT_EQ(app_data()->status(), WebKioskAppData::Status::kInit);
Expand All @@ -329,38 +340,40 @@ TEST_F(WebKioskAppUpdateObserverTest, ShouldNotUpdateAppInfoForPlaceholders) {
// Install app as placeholder.
std::string app_id;
{
base::RunLoop loop;
EXPECT_CALL(*mock_app_registry_observer(), OnAppUpdate);
std::unique_ptr<web_app::WebApp> web_app = web_app::test::CreateWebApp(
GURL(kAppLaunchUrl), web_app::WebAppManagement::kKiosk);
web_app->SetName(kAppTitle);
app_id = web_app->app_id();
std::unique_ptr<web_app::WebAppRegistryUpdate> update =
sync_bridge().BeginUpdate();
update->CreateApp(std::move(web_app));
sync_bridge().CommitUpdate(
std::move(update),
base::BindLambdaForTesting([app_id, &loop](bool success) {
ASSERT_TRUE(success);
loop.QuitWhenIdle();
}));

// TODO(crbug/1379290): Use `TestFuture<void>` in all these tests
EXEC_AND_WAIT_FOR_CALL(
{
TestFuture<bool> success_future;
sync_bridge().CommitUpdate(std::move(update),
success_future.GetCallback());
EXPECT_TRUE(success_future.Get());
},
*mock_app_registry_observer(), OnAppUpdate);

web_app::test::AddInstallUrlAndPlaceholderData(
profile()->GetPrefs(), &sync_bridge(), app_id, GURL(kAppInstallUrl),
web_app::ExternalInstallSource::kKiosk, /*is_placeholder=*/true);
loop.Run();
}

// Update app info.
{
base::RunLoop loop;
EXPECT_CALL(*mock_app_registry_observer(), OnAppUpdate)
.WillOnce(RunClosure(loop.QuitClosure()));
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->app_id = app_id;
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
loop.Run();
EXEC_AND_WAIT_FOR_CALL(
{
std::vector<apps::AppPtr> apps;
apps.push_back(CreateTestApp());
apps[0]->app_id = app_id;
app_service()->OnApps(std::move(apps), apps::AppType::kWeb,
/*should_notify_initialized=*/true);
},
*mock_app_registry_observer(), OnAppUpdate);
}

EXPECT_EQ(app_data()->status(), WebKioskAppData::Status::kInit);
Expand Down

0 comments on commit ab7843c

Please sign in to comment.