[Merge] lp:~ken-vandine/content-hub/process_cpp into lp:content-hub

Michael Sheldon michael.sheldon at canonical.com
Thu Jun 9 13:16:43 UTC 2016


Review: Needs Information

Couple of questions in comments, but otherwise this looks good to me :)

Diff comments:

> 
> === modified file 'tests/acceptance-tests/app_hub_communication_handler.cpp'
> --- tests/acceptance-tests/app_hub_communication_handler.cpp	2015-10-20 20:23:09 +0000
> +++ tests/acceptance-tests/app_hub_communication_handler.cpp	2016-06-09 13:03:21 +0000
> @@ -135,27 +150,20 @@
>          test::TestHarness harness;
>          harness.add_test_case([default_peer_id, default_dest_peer_id]()
>          {
> -            /* register handler on the service */
> -            auto mock_handler = new MockedHandler{};
> -            EXPECT_CALL(*mock_handler, handle_export(_)).Times(Exactly(1));
> -            qputenv("APP_ID", default_peer_id.toLatin1());
> +            qputenv("APP_ID", default_dest_peer_id.toLatin1());
>              auto hub = cuc::Hub::Client::instance();
> -            hub->register_import_export_handler(mock_handler);
> -            hub->quit();
> -
> -            qputenv("APP_ID", default_dest_peer_id.toLatin1());
>              hub = cuc::Hub::Client::instance();
>              auto transfer = hub->create_import_from_peer(cuc::Peer(default_peer_id));
> -            ASSERT_TRUE(transfer != nullptr);
> +            EXPECT_TRUE(transfer != nullptr);
>              EXPECT_TRUE(transfer->start());
>              EXPECT_EQ(cuc::Transfer::in_progress, transfer->state());
> -            EXPECT_EQ(cuc::Transfer::charged, transfer->state());
> +            //EXPECT_EQ(cuc::Transfer::charged, transfer->state());

Should this be commented out?

>              hub->quit();
> -            delete mock_handler;
>          });
>  
>          EXPECT_EQ(0, QTest::qExec(std::addressof(harness)));
> +        return ::testing::Test::HasFailure() ? core::posix::exit::Status::failure : core::posix::exit::Status::success;
>      };
>  
> -    EXPECT_TRUE(test::fork_and_run(child, parent) != EXIT_FAILURE);
> +    EXPECT_EQ(core::testing::ForkAndRunResult::empty, core::testing::fork_and_run(service, client));
>  }
> 
> === modified file 'tests/acceptance-tests/app_hub_communication_transfer.cpp'
> --- tests/acceptance-tests/app_hub_communication_transfer.cpp	2015-10-20 20:23:09 +0000
> +++ tests/acceptance-tests/app_hub_communication_transfer.cpp	2016-06-09 13:03:21 +0000
> @@ -151,47 +158,33 @@
>              auto hub = cuc::Hub::Client::instance();
>              auto transfer = hub->create_import_from_peer(
>                  hub->default_source_for_type(cuc::Type::Known::pictures()));
> -            ASSERT_TRUE(transfer != nullptr);
> +            EXPECT_TRUE(transfer != nullptr);
>              EXPECT_EQ(cuc::Transfer::created, transfer->state());
> -            ASSERT_FALSE(hub->has_pending(transfer->destination()));
> +            EXPECT_FALSE(hub->has_pending(transfer->destination()));
>              EXPECT_TRUE(transfer->setSelectionType(cuc::Transfer::SelectionType::multiple));
> -            ASSERT_EQ(cuc::Transfer::SelectionType::multiple, transfer->selectionType());
> +            EXPECT_EQ(cuc::Transfer::SelectionType::multiple, transfer->selectionType());
>              transfer->setStore(new cuc::Store{store_dir.path()});
>              EXPECT_TRUE(transfer->start());
>              EXPECT_EQ(cuc::Transfer::initiated, transfer->state());
> -            ASSERT_TRUE(hub->has_pending(transfer->destination()));
> +            EXPECT_TRUE(hub->has_pending(transfer->destination()));
>              EXPECT_TRUE(transfer->setSelectionType(cuc::Transfer::SelectionType::single));
> -            ASSERT_EQ(cuc::Transfer::SelectionType::multiple, transfer->selectionType());
> +            EXPECT_EQ(cuc::Transfer::SelectionType::multiple, transfer->selectionType());
>              EXPECT_TRUE(transfer->charge(source_items));
>              EXPECT_EQ(cuc::Transfer::charged, transfer->state());
>              EXPECT_EQ(expected_items, transfer->collect());
>              /** [Importing pictures] */
>  
> -            /** Test that the transfer aborts when destination file exists */
> -            auto dupe_transfer = hub->create_import_from_peer(
> -                hub->default_source_for_type(cuc::Type::Known::pictures()));
> -            ASSERT_TRUE(dupe_transfer != nullptr);
> -            EXPECT_EQ(cuc::Transfer::created, dupe_transfer->state());
> -            EXPECT_TRUE(dupe_transfer->setSelectionType(cuc::Transfer::SelectionType::multiple));
> -            ASSERT_EQ(cuc::Transfer::SelectionType::multiple, dupe_transfer->selectionType());
> -            dupe_transfer->setStore(new cuc::Store{store_dir.path()});
> -            EXPECT_TRUE(dupe_transfer->start());
> -            EXPECT_EQ(cuc::Transfer::initiated, dupe_transfer->state());
> -            EXPECT_TRUE(dupe_transfer->charge(source_items));
> -            EXPECT_EQ(cuc::Transfer::aborted, dupe_transfer->state());
> -            /* end dest exists test */
> -

Are these removed because we handle filename collisions within content-hub automatically now?

>              /* Test that only a single transfer exists for the same peer */
>              auto single_transfer = hub->create_import_from_peer(
>                  hub->default_source_for_type(cuc::Type::Known::pictures()));
> -            ASSERT_TRUE(single_transfer != nullptr);
> +            EXPECT_TRUE(single_transfer != nullptr);
>              EXPECT_EQ(cuc::Transfer::created, single_transfer->state());
>              EXPECT_TRUE(single_transfer->start());
>              EXPECT_EQ(cuc::Transfer::initiated, single_transfer->state());
>  
>              auto second_transfer = hub->create_import_from_peer(
>                  hub->default_source_for_type(cuc::Type::Known::pictures()));
> -            ASSERT_TRUE(second_transfer != nullptr);
> +            EXPECT_TRUE(second_transfer != nullptr);
>              EXPECT_EQ(cuc::Transfer::created, second_transfer->state());
>              /* Now that a second transfer was created, the previous
>               * transfer should have been aborted */


-- 
https://code.launchpad.net/~ken-vandine/content-hub/process_cpp/+merge/296935
Your team Ubuntu Phablet Team is subscribed to branch lp:content-hub.



More information about the Ubuntu-reviews mailing list