Merge lp:~mvo/unity-scope-click/empty-frameworks-query into lp:unity-scope-click/devel

[Michael Vogt]

This branch sends a empty "framework:" string if the system has no frameworks installed.

The search scope will return all apps for the given search query if the ",framework:" is missing but when the user tries to install the click app click will fail.

I ran into this when testing unity8-desktop-session-mir. If you feel like this should be fixed on the server side instead, please simply close this MP.

[lp:~mvo/unity-scope-click/empty-frameworks-query]

264. By Michael Vogt on 2014-05-19

improve test

265. By Michael Vogt on 2014-05-19

HACKING: mention how to run a specific test only

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:263
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~mvo/unity-scope-click/empty-frameworks-query/+merge/220083/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/71/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/46
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/45
        deb: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/45/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/45

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/71/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:265
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~mvo/unity-scope-click/empty-frameworks-query/+merge/220083/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/72/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/47/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/46/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/46/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-team-unity-scope-click-devel-ci/72/rebuild

review: Needs Fixing (continuous-integration)

[dobey (dobey)]

I'm voting Disapprove here, because it directly conflicts with your other branch https://code.launchpad.net/~mvo/unity-scope-click/lp1292645-use-libclick/+merge/220061 and I'm not sure it's the solution we should be going for here.

Also, please file a new bug for this issue where results are given, but not installable, when there are no frameworks. As the frameworks data is a core piece of information we depend on for a working scope, I think we should fail when they are none available, and have a runtime dependency on the package that provides them (ubuntu-sdk-libs).

review: Disapprove

[Michael Vogt (mvo)]

Thanks Rodney for your review. I created bug #1320975 as requested.

I don't really see the conflict with my other branch here, configuration->get_available_frameworks() may still return a empty array (but maybe I'm missing something).

But if there is going to be a dependency on the ubuntu-sdk-libs package this branch will not be needed.

[dobey (dobey)]

On Mon, 2014-05-19 at 18:43 +0000, Michael Vogt wrote:
> I don't really see the conflict with my other branch here,
> configuration->get_available_frameworks() may still return a empty
> array (but maybe I'm missing something).

Oh, sorry. I was reading the change in index.cpp here, and
configuration.cpp there, as the same code. :)

This branch does conflict with the tip of /devel now though. But I still
think this isn't how we should fix the issue, and instead we should have
a proper dependency on the data from other packages we need, and should
fail appropriately rather than trying to pass empty data to the server.
Although, I think tip of /devel will pass empty data to the server now
anyway, as we're passing an empty header, rather than not passing
frameworks at all, in the empty list case.

Unmerged revisions

265. By Michael Vogt on 2014-05-19

HACKING: mention how to run a specific test only

264. By Michael Vogt on 2014-05-19

improve test

263. By Michael Vogt on 2014-05-19

send empty frameworks string if no frameworks are installed

=== modified file 'HACKING'
--- HACKING	2014-05-16 21:08:36 +0000
+++ HACKING	2014-05-19 15:22:38 +0000
@@ -37,6 +37,10 @@
 
     $ make check-valgrind
 
+Running a specific test with:
+
+    $ ./scope/tests/click-scope-tests  --gtest_filter=QueryStringTest.testBuildQueryNoInstalledFrameworks
+
 
 Running the autopilot tests
 ---------------------------

=== modified file 'scope/click/index.cpp'
--- scope/click/index.cpp	2014-04-16 21:13:28 +0000
+++ scope/click/index.cpp	2014-05-19 15:22:38 +0000
@@ -92,7 +92,10 @@
     std::stringstream result;
 
     result << query;
-    for (auto f: configuration->get_available_frameworks()) {
+    auto available_frameworks = configuration->get_available_frameworks();
+    if (available_frameworks.size() == 0)
+       result << ",framework:";
+    for (auto f: available_frameworks) {
         result << ",framework:" << f;
     }
     result << ",architecture:" << configuration->get_architecture();

=== modified file 'scope/click/index.h'
--- scope/click/index.h	2014-04-16 21:13:28 +0000
+++ scope/click/index.h	2014-05-19 15:22:38 +0000
@@ -46,7 +46,6 @@
 const std::string SEARCH_BASE_URL_ENVVAR = "U1_SEARCH_BASE_URL";
 const std::string SEARCH_BASE_URL = "https://search.apps.ubuntu.com/";
 const std::string SEARCH_PATH = "api/v1/search";
-const std::string SUPPORTED_FRAMEWORKS = "framework:ubuntu-sdk-13.10";
 const std::string QUERY_ARGNAME = "q";
 const std::string ARCHITECTURE = "architecture:";
 const std::string DETAILS_PATH = "api/v1/package/";

=== modified file 'scope/tests/test_index.cpp'
--- scope/tests/test_index.cpp	2014-04-29 03:17:38 +0000
+++ scope/tests/test_index.cpp	2014-05-19 15:22:38 +0000
@@ -496,3 +496,11 @@
     EXPECT_NE(std::string::npos, index_query.find("framework:" + fake_fwk_1));
     EXPECT_NE(std::string::npos, index_query.find("framework:" + fake_fwk_2));
 }
+
+TEST_F(QueryStringTest, testBuildQueryNoInstalledFrameworks)
+{
+    EXPECT_CALL(*configPtr, get_architecture()).Times(1).WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks()).Times(1).WillOnce(Return(std::vector<std::string>()));
+    auto index_query = indexPtr->build_index_query("fake");
+    EXPECT_EQ("fake,framework:,architecture:fake_arch", index_query);
+}