Merge lp:~stolowski/unity-scope-home/phone-disable-filters into lp:unity-scope-home

[Paweł Stołowski]

Disable filter updates if phone factor is not 'desktop'.

Disable filter updates if phone factor is not 'desktop' (i.e. currently displayed categores are not reflected in the filters of Home view). This is required because UI in Unity 8 support single selection filters only (any selection made by the user will narrow results to single category in Home).

In addition, sort scope registry by scope id to give more sensible default order on the phone, until real category sorting works there.

[Michal Hruby (mhr3)]

Could we see some tests for this? Not updating the filters seems a bit scary, does changing the query still reset them etc? Those things should be ensured by a test.

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:170
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/91/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/55
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/55
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/55

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/91/rebuild

review: Approve (continuous-integration)

[Paweł Stołowski (stolowski)]

> Could we see some tests for this? Not updating the filters seems a bit scary,
> does changing the query still reset them etc? Those things should be ensured
> by a test.

Right, that was a bug; fixed.

As much as I'd love to add a test for it, I don't see a way to do it with a reasonable amount of effort due to the number of prerequisites or mocks needed.

The point of this change is to disable updates of filters in the UI from the backend (which only Home Scope does). The logic that sets scopes to query from SSS recommendations, always-search scopes or default view stays.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:171
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/92/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/56
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/56
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/56

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/92/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:176
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/98/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/62
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/62
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/62

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/98/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:177
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/99/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/63

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/99/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:179
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/100/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/64
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/64
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/64

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/100/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:180
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/101/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/65
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/65
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/65/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/101/rebuild

review: Needs Fixing (continuous-integration)

[Michal Hruby (mhr3)]

222 + echo $(nodist_test_home_scope_SOURCES)

Would be nice to get rid of it :)

418 + Process.close_pid (server_pid);

I think this frees the handle on some systems, yet it's used later in stop(), fortunately it's a noop on linux, anyway, better remove it (or move to destructor to have super-clean code).

519 + var proxy = acquire_test_proxy (HOME_SCOPE_DBUS_NAME, HOME_SCOPE_DBUS_PATH);
520 + var channel_id = open_channel (proxy, ChannelType.GLOBAL, null);
521 +
522 + assert (channel_id != null);

Looks like something that should go into setup() + close_channel() in teardown().

80 + string? form_factor = scope_search.search_context.search_metadata.form_factor;

Micro-optimisation - turn this into unowned string.

I know this was a hard fight, but it really opens doors to have more thorough testing of the entire home-scope codebase... Well done! :)

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:182
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/102/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/66
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/66
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/66

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/102/rebuild

review: Approve (continuous-integration)

[Michal Hruby (mhr3)]

+1

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:183
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/103/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/67
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/67
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/67

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/103/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-scope-home-autolanding/74/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-autolanding/44/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-autolanding/44
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-autolanding/44

review: Needs Fixing (continuous-integration)

[Francis Ginther (fginther)]

Autolanding failure caused by catching the archive in the middle of an update (hash sum mismatch). Re-approved.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'configure.ac'
--- configure.ac	2013-09-27 14:33:11 +0000
+++ configure.ac	2013-10-01 10:06:09 +0000
@@ -104,7 +104,7 @@
 ####################################################################
 AC_ARG_ENABLE([headless-tests],
               AS_HELP_STRING([--enable-headless-tests=@<:@no/yes@:>@],[enable headless test suite (requires Xvfb) @<:@default=no@:>@]),,
-              [enable_headless_tests=no])
+              [enable_headless_tests=yes])
 
 AM_CONDITIONAL([ENABLE_HEADLESS_TESTS],[test "x$enable_headless_tests" != "xno"])
 
 
=== modified file 'src/platform-info.vala'
--- src/platform-info.vala	2013-08-28 14:24:07 +0000
+++ src/platform-info.vala	2013-10-01 10:06:09 +0000
@@ -92,17 +92,20 @@
       try
       {
         is_ready = false;
-        var connection = yield Bus.get (BusType.SYSTEM, null);
-        var reply = yield connection.call ("org.ofono", "/ril_0",
-                                           "org.ofono.SimManager",
-                                           "GetProperties", null,
-                                           new VariantType ("(a{sv})"),
-                                           DBusCallFlags.NONE, -1, null);
-        reply = reply.get_child_value (0);
-        var mcc_v = reply.lookup_value ("MobileCountryCode", VariantType.STRING);
-        var mnc_v = reply.lookup_value ("MobileNetworkCode", VariantType.STRING);
-        if (mcc_v != null) country_code = mcc_v.get_string ();
-        if (mnc_v != null) network_code = mnc_v.get_string ();
+        if (Environment.get_variable ("HOME_SCOPE_IGNORE_OFONO") == null)
+        {
+            var connection = yield Bus.get (BusType.SYSTEM, null);
+            var reply = yield connection.call ("org.ofono", "/ril_0",
+                                               "org.ofono.SimManager",
+                                               "GetProperties", null,
+                                               new VariantType ("(a{sv})"),
+                                               DBusCallFlags.NONE, -1, null);
+            reply = reply.get_child_value (0);
+            var mcc_v = reply.lookup_value ("MobileCountryCode", VariantType.STRING);
+            var mnc_v = reply.lookup_value ("MobileNetworkCode", VariantType.STRING);
+            if (mcc_v != null) country_code = mcc_v.get_string ();
+            if (mnc_v != null) network_code = mnc_v.get_string ();
+        }
       }
       catch (Error err)
       {
 
=== modified file 'src/scope-registry.vala'
--- src/scope-registry.vala	2013-09-04 10:28:06 +0000
+++ src/scope-registry.vala	2013-10-01 10:06:09 +0000
@@ -119,7 +119,8 @@
           // with remote scopes; we would need to wait for remote-scopes query to finish to know if
           // if this master is needed for remote results.
 
-          this.registry.append (node);
+          // sort master scopes by scope_id
+          this.registry.insert_sorted (node, (node_a, node_b) => { return strcmp (node_a.scope_info.id, node_b.scope_info.id); });
           master_to_node.insert (node.scope_info.id, node);
         }
         else
 
=== modified file 'src/scope.vala'
--- src/scope.vala	2013-09-20 16:40:39 +0000
+++ src/scope.vala	2013-10-01 10:06:09 +0000
@@ -42,7 +42,7 @@
   private FilterState filter_state = new FilterState ();
   private KeywordSearch keywords_search = new KeywordSearch ();
   private bool smart_scopes_initialized = false;
-  private bool smart_scopes_ready = false;
+  public bool smart_scopes_ready { get; internal set; default = false; }
   private SmartScopes.SmartScopeClientInterface sss_client = null;
   private SmartScopes.ChannelIdMap channel_id_map = new SmartScopes.ChannelIdMap ();
   private uint metrics_timer;
@@ -616,6 +616,13 @@
     uint num_scopes = 0;
 
     bool flushing_enabled = false;
+    unowned string? form_factor = scope_search.search_context.search_metadata.form_factor;
+    if (form_factor == null)
+        form_factor = "unknown"; // set to 'unknown' as it's sent with smart scopes request
+    bool disable_filter_updates = (form_factor != "desktop");
+    
+    if (disable_filter_updates)
+      debug ("Filter updates disabled, form factor is %s", form_factor);
 
     // ids of scopes recommended by Smart Scope Service
     var recommended_search_scopes = new List<SmartScopes.RecommendedScope?> ();
@@ -711,6 +718,8 @@
       unowned Unity.OptionsFilter categories_filter = scope_search.get_filter ("categories") as Unity.OptionsFilter;
       unowned Unity.OptionsFilter sources_filter = scope_search.get_filter ("sources") as Unity.OptionsFilter;
 
+      bool relations_changed = false;
+        
       // if query is empty but it's not just a filter change (i.e. state is REMOVES_FROM_PREVIOUS_QUERY or NEW_QUERY),
       // then apply default user filters.
       if (empty_query && search_query_changed != SearchQueryChange.NOT_CHANGED)
@@ -724,8 +733,12 @@
         default_view = true;
       }
 
-      debug ("Updating filter interrelationships");
-      bool relations_changed = filter_state.update_filter_relations (scope_search.channel_id, categories_filter, sources_filter);
+      if (!disable_filter_updates) {
+        debug ("Updating filter interrelationships");
+        relations_changed = filter_state.update_filter_relations (scope_search.channel_id, categories_filter, sources_filter);
+      } else {
+        needs_filter_update = false; //reset filter update flag if on the phone
+      }
 
       // caution: push_filter_settings may get cancelled if we ever yield before;
       // in such case internal filter state must be updated later (at the end of search).
@@ -888,7 +901,6 @@
           }
 
           sss_query_started = true;
-          var form_factor = SearchUtil.get_form_factor (scope_search);
           sss_client.search.begin (search_string, form_factor, session_id, remote_scopes_to_query, scope_mgr.disabled_scopes,
                                    (scope_id, row) =>
             {
@@ -1024,7 +1036,7 @@
       use_recommended_scopes = true;
 
     // only update filters with searched scopes when not in the default view, otherwise default view filters may get de-selected
-    if (!default_view)
+    if (!default_view && !disable_filter_updates)
     {
       debug ("Updating filter state");
       needs_filter_update |= SearchUtil.update_filters (search_scopes, use_recommended_scopes ? recommended_search_scopes : null,
@@ -1108,7 +1120,7 @@
     }
 
     // update filter state again, but this time check result counts and send the update only if any of the highlighted masters has no results
-    if (!default_view)
+    if (!default_view && !disable_filter_updates)
       needs_filter_update |= SearchUtil.update_filters (search_scopes, use_recommended_scopes ? recommended_search_scopes : null, scope_search, filter_change_only == false);
 
     if (needs_filter_update)
 
=== modified file 'src/search-util.vala'
--- src/search-util.vala	2013-06-14 15:49:21 +0000
+++ src/search-util.vala	2013-10-01 10:06:09 +0000
@@ -317,15 +317,4 @@
     }
     return found;
   }
-
-  public string get_form_factor (Unity.AggregatedScopeSearch scope_search)
-  {
-    const string FORM_FACTOR_HINT = "form-factor";
-    unowned Variant? hint = scope_search.hints[FORM_FACTOR_HINT];
-    if (hint != null && hint.is_of_type (VariantType.STRING))
-    {
-      return hint.get_string ();
-    }
-    return "unknown";
-  }
 }
 
=== added file 'tests/fake-server/samples/remote-scopes-minimal.txt'
--- tests/fake-server/samples/remote-scopes-minimal.txt	1970-01-01 00:00:00 +0000
+++ tests/fake-server/samples/remote-scopes-minimal.txt	2013-10-01 10:06:09 +0000
@@ -0,0 +1,1 @@
+[{"description": "This is an Ubuntu search plugin that enables information from Ubuntu One Music Store to be searched and displayed in the Dash underneath the More Suggestions header. If you do not wish to search this content source, you can disable this search plugin.\n", "screenshot": "http://ubuntuone.com/7XpGFTkSugUON0PEsBBYHK", "name": "Ubuntu One Music Search", "keywords": ["u1ms", "u1", "ubuntuone"], "id": "more_suggestions-u1ms.scope", "icon": "file:///usr/share/icons/unity-icon-theme/places/svg/service-u1.svg"}, {"description": "This is an Ubuntu search plugin that enables information from Skimlinks to be searched and displayed in the Dash underneath the More Suggestions header. If you do not wish to search this content source, you can disable this search plugin.\n", "screenshot": "http://ubuntuone.com/3VvxS1Dri9CKKloEzBs2lT", "name": "Skimlinks", "keywords": ["skimlinks"], "id": "more_suggestions-skimlinks.scope", "icon": "file:///usr/share/icons/unity-icon-theme/places/svg/group-treat-yourself.svg"}, {"description": "This is an Ubuntu search plugin that enables information from Wikipedia to be searched and displayed in the Dash underneath the Reference header. If you do not wish to search this content source, you can disable this search plugin.\n", "screenshot": "http://ubuntuone.com/4QiCFcHJS3fmDCMFvgi9lo", "name": "Wikipedia", "keywords": ["wikipedia", "wiki"], "id": "reference-wikipedia.scope", "icon": "file:///usr/share/icons/unity-icon-theme/places/svg/service-wikipedia.svg"}, {"description": "This is an Ubuntu search plugin that enables information from Wordnik to be searched and displayed in the Dash underneath the Reference header. If you do not wish to search this content source, you can disable this search plugin.\n", "screenshot": "http://ubuntuone.com/15gptcYAwD6vk53cUtOrvX", "name": "Dictionary", "keywords": ["define"], "id": "reference-dictionary.scope", "icon": "file:///usr/share/icons/unity-icon-theme/places/svg/service-wordnik.svg"}, {"description": "Find themoviedb.org items", "screenshot": null, "name": "themoviedb.org", "keywords": ["themoviedb", "movie", "db", "actor", "cinema"], "id": "reference-themoviedb.scope", "icon": null}, {"description": "This is an Ubuntu search plugin that enables information from Amazon to be searched and displayed in the Dash underneath the More Suggestions header. If you do not wish to search this content source, you can disable this search plugin.\n", "screenshot": "http://ubuntuone.com/5apoKe0t8OOj1BQkWKnfWi", "name": "Amazon", "keywords": ["amazon"], "id": "more_suggestions-amazon.scope", "icon": "file:///usr/share/icons/unity-icon-theme/places/svg/service-amazon.svg"}, {"description": "This is an Ubuntu search plugin that enables information from Etsy to be searched and displayed in the Dash underneath the More Suggestions header. If you do not wish to search this content source, you can disable this search plugin.", "screenshot": null, "name": "Etsy", "keywords": ["etsy"], "id": "more_suggestions-etsy.scope", "icon": null}, {"description": "This is an Ubuntu search plugin that enables information from eBay to be searched and displayed in the Dash underneath the More Suggestions header. If you do not wish to search this content source, you can disable this search plugin.", "screenshot": null, "name": "eBay", "keywords": ["ebay"], "id": "more_suggestions-ebay.scope", "icon": null}, {"description": "Find Stackexchange items", "screenshot": null, "name": "Stackexchange", "keywords": ["stackexchange"], "id": "reference-stackexchange.scope", "icon": null}]
 
=== modified file 'tests/unit/Makefile.am'
--- tests/unit/Makefile.am	2013-06-11 15:36:38 +0000
+++ tests/unit/Makefile.am	2013-10-01 10:06:09 +0000
@@ -19,6 +19,7 @@
 	  --pkg glib-2.0 \
 	  --pkg json-glib-1.0 \
 	  --pkg gee-1.0 \
+	  --pkg unity-extras \
 	  --vapidir $(srcdir) \
 	  --vapidir $(top_srcdir)/vapi \
 	  --target-glib=2.26 \
@@ -36,7 +37,6 @@
 
 AM_CPPFLAGS = \
   -DDATADIR=\"$(DATADIR)\" \
-  -DPKGDATADIR=\"$(PKGDATADIR)\" \
   -DGETTEXT_PACKAGE=\"$(GETTEXT_PACKAGE)\" \
   -DG_LOG_DOMAIN=\"unity-scope-home\" \
   $(HOME_SCOPE_CFLAGS) \
@@ -49,7 +49,7 @@
   AM_CPPFLAGS += -w
 endif
 
-test_home_scope_VALASOURCES = \
+MAINSOURCES = \
 	../../src/scope.vala \
 	../../src/scope-manager.vala \
 	../../src/keyword-search.vala \
@@ -74,11 +74,21 @@
 	../../src/markup-cleaner.vala \
 	../../src/master-scopes.vala \
 	../../src/smart-scopes-preview-parser.vala \
+	$(NULL)
+
+TEST_SOURCES = \
 	config-tests.vala \
+	test-utils.vala \
 	test-home-scope.vala \
 	$(NULL)
 
-nodist_test_home_scope_SOURCES = $(test_home_scope_VALASOURCES:.vala=.c)
+test_home_scope_VALASOURCES = $(TEST_SOURCES) $(MAINSOURCES)
+
+# the patsubst rule below and objects override are needed to force dependency on *.c files
+# generated in current test dir rather than on those from ../../src
+nodist_test_home_scope_SOURCES = $(TEST_SOURCES:.vala=.c) $(patsubst ../../src/%,%,$(MAINSOURCES:.vala=.c))
+nodist_test_home_scope_OBJECTS = $(nodist_test_home_scope_SOURCES:.c=.o)
+
 
 CLEANFILES = *.stamp \
 	  *.c \
@@ -96,6 +106,7 @@
 	$(AM_V_GEN)$(VALAC) -C $(AM_VALAFLAGS) $(VALAFLAGS) $^
 	@touch $@
 
+
 # START HEADLESS TESTS
 if ENABLE_HEADLESS_TESTS
 test-headless:
 
=== renamed file 'tests/unit/data/client-scopes.json' => 'tests/unit/data/unity/client-scopes.json'
=== modified file 'tests/unit/data/unity/scopes/masterscope_a.scope'
--- tests/unit/data/unity/scopes/masterscope_a.scope	2013-01-23 10:48:55 +0000
+++ tests/unit/data/unity/scopes/masterscope_a.scope	2013-10-01 10:06:09 +0000
@@ -1,6 +1,6 @@
 [Scope]
-DBusName=com.canonical.Unity.Scope.MasterA
-DBusPath=/com/canonical/unity/scope/mastera
+DBusName=com.canonical.Unity.Scope.HomeTest
+DBusPath=/com/canonical/unity/masterscopes/mastera
 Icon=/usr/share/unity/6/icon1.svg
 IsMaster=true
 RequiredMetadata=some_id[s];foo[s];bar[y];
 
=== modified file 'tests/unit/data/unity/scopes/masterscope_b.scope'
--- tests/unit/data/unity/scopes/masterscope_b.scope	2013-01-23 10:48:55 +0000
+++ tests/unit/data/unity/scopes/masterscope_b.scope	2013-10-01 10:06:09 +0000
@@ -1,6 +1,6 @@
 [Scope]
-DBusName=com.canonical.Unity.Scope.MasterB
-DBusPath=/com/canonical/unity/scope/masterb
+DBusName=com.canonical.Unity.Scope.HomeTest
+DBusPath=/com/canonical/unity/masterscopes/masterb
 Icon=/usr/share/unity/6/icon2.svg
 IsMaster=true
 RequiredMetadata=bid[s];a[s];b[y];
 
=== added file 'tests/unit/data/unity/scopes/more_suggestions.scope'
--- tests/unit/data/unity/scopes/more_suggestions.scope	1970-01-01 00:00:00 +0000
+++ tests/unit/data/unity/scopes/more_suggestions.scope	2013-10-01 10:06:09 +0000
@@ -0,0 +1,17 @@
+[Scope]
+DBusName=com.canonical.Unity.Scope.HomeTest
+DBusPath=/com/canonical/unity/masterscope/moresuggestions
+Icon=
+CategoryIcon=/usr/share/icons/unity-icon-theme/places/svg/group-treat-yourself.svg
+Name=More suggestions
+Type=moresug
+IsMaster=true
+
+[Desktop Entry]
+X-Ubuntu-Gettext-Domain=unity-scope-home
+
+[Category more_suggestions]
+Name=More suggestions
+Icon=
+SortField=uri
+
 
=== added file 'tests/unit/data/unity/scopes/reference.scope'
--- tests/unit/data/unity/scopes/reference.scope	1970-01-01 00:00:00 +0000
+++ tests/unit/data/unity/scopes/reference.scope	2013-10-01 10:06:09 +0000
@@ -0,0 +1,33 @@
+[Scope]
+DBusName=com.canonical.Unity.Scope.HomeTest
+DBusPath=/com/canonical/unity/masterscope/reference
+Icon=
+CategoryIcon=/usr/share/icons/unity-icon-theme/places/svg/group-reference.svg
+Name=Reference
+Type=reference
+IsMaster=true
+Keywords=reference;
+
+[Desktop Entry]
+X-Ubuntu-Gettext-Domain=unity-scope-home
+
+[Category encyclopedia]
+Name=Encyclopedia
+Icon=
+DedupField=uri
+
+[Category vocabulary]
+Name=Vocabulary
+Icon=
+DedupField=uri
+
+[Category scholar]
+Name=Scholar
+Icon=
+DedupField=uri
+
+[Category sources]
+Name=Sources
+Icon=
+DedupField=uri
+
 
=== modified file 'tests/unit/test-home-scope.vala'
--- tests/unit/test-home-scope.vala	2013-09-19 15:22:33 +0000
+++ tests/unit/test-home-scope.vala	2013-10-01 10:06:09 +0000
@@ -17,55 +17,77 @@
  */
 
 using Unity.HomeScope.SmartScopes;
+using Unity.Protocol;
 
 namespace Unity.HomeScope
 {
 
+  static const string HOME_SCOPE_DBUS_NAME = "com.canonical.Unity.Scope.HomeTest";
+  static const string HOME_SCOPE_DBUS_PATH = "/com/canonical/unity/home";
+
   bool verbose_debug = false;
+
+  HomeScope? scope = null;
+  Application? app = null;
   
-  /* A bit of magic to get proper-ish fixture support */
-  public interface Fixture : Object
-  {
-    class DelegateWrapper
-    {
-      TestDataFunc func;
-      public DelegateWrapper (owned TestDataFunc f) { func = (owned) f; }
-    }
-
-    public virtual void setup () {}
-    public virtual void teardown () {}
-
-    [CCode (has_target = false)]
-      public delegate void Callback<T> (T ptr);
-
-    private static List<DelegateWrapper> _tests;
-
-    public static unowned TestDataFunc create<F> (Callback<void*> cb)
-      requires (typeof (F).is_a (typeof (Fixture)))
-      {
-        TestDataFunc functor = () =>
-        {
-          var type = typeof (F);
-          var instance = Object.new (type) as Fixture;
-          instance.setup ();
-          cb (instance);
-          instance.teardown ();
-        };
-        unowned TestDataFunc copy = functor;
-        _tests.append (new DelegateWrapper ((owned) functor));
-        return copy;
-      }
-    public static unowned TestDataFunc create_static<F> (Callback<F> cb)
-    {
-      return create<F> ((Callback<void*>) cb);
-    }
-  }
-
   private KeywordSearch kw;
   const string[] RESULTS_SCHEMA = {"s", "s", "u", "u", "s", "s", "s", "s", "a{sv}"}; //TODO use schema def from libunity when it's public
 
+  public class FakeSmartScopesServer
+  {
+    private Rand rand = new Rand ();
+    private Pid server_pid;
+    public int server_port { get; internal set; }
+
+    public void start () throws SpawnError
+    {
+      server_port = rand.int_range (1024, 9000); 
+    
+      Process.spawn_async (null, {Config.TOPSRCDIR + "/tests/fake-server/fake-sss-server.py",
+          "--scopes", Config.TOPSRCDIR + "/tests/fake-server/samples/remote-scopes-minimal.txt",
+          "--search", Config.TESTRUNDATADIR + "/search.dump",
+          "--feedback", Config.TESTRUNDATADIR + "/feedback.dump",
+          "--requests", "6",
+          "--timeout", "15",
+          "--port", server_port.to_string (),
+          Config.TOPSRCDIR + "/tests/unit/data/search_results1.txt"},
+      null, 0, null, out server_pid);
+    
+      Socket socket = new Socket (SocketFamily.IPV4, SocketType.STREAM, SocketProtocol.TCP);
+      assert (socket != null);
+    
+      InetAddress addr = new InetAddress.from_bytes ({127, 0, 0, 1}, SocketFamily.IPV4);
+      InetSocketAddress server_addr = new InetSocketAddress (addr, (uint16)server_port);
+      
+      bool conn = false;
+      int retry = 5;
+      while (!conn && retry > 0)
+      {
+        try
+        {
+          conn = socket.connect (server_addr);
+        }
+        catch (Error e) {}
+        if (!conn)
+          Thread.usleep (1*1000000); // sleep for 1 second
+        --retry;
+      }
+    }
+
+    public void stop ()
+    {
+      Posix.kill (server_pid, Posix.SIGTERM);
+      Process.close_pid (server_pid);
+    }
+  }
+
+  FakeSmartScopesServer fake_server;
+
   public static int main (string[] args)
   {
+    Environment.set_variable ("GSETTINGS_BACKEND", "memory", true);
+    Environment.set_variable ("HOME_SCOPE_IGNORE_OFONO", "1", true);
+
     var xdg_data_dirs = Environment.get_variable ("XDG_DATA_DIRS");
     if (xdg_data_dirs == null) xdg_data_dirs = "/usr/share";
     Environment.set_variable ("XDG_DATA_DIRS",
@@ -74,10 +96,14 @@
     Environment.set_variable ("LIBUNITY_SCOPE_DIRECTORIES",
                               "%s/unity/scopes".printf (Config.TESTDATADIR),
                               true);
+
+    fake_server = new FakeSmartScopesServer ();
+    fake_server.start ();
+
     kw = new KeywordSearch ();
 
     Test.init (ref args); 
-
+    
     Test.add_data_func ("/Unit/Search/ScopeRegistry", Fixture.create<HomeScopeSearchTester> (HomeScopeSearchTester.test_registry));
     Test.add_data_func ("/Unit/Search/KeywordSearch", Fixture.create<HomeScopeSearchTester> (HomeScopeSearchTester.test_keyword_search));
     Test.add_data_func ("/Unit/Search/SearchQueryState", Fixture.create<HomeScopeSearchTester> (HomeScopeSearchTester.test_search_query_state));
@@ -131,37 +157,153 @@
     Test.add_data_func ("/Unit/MarkupCleaner/UnsupportedEntitiesAreRaw", Fixture.create<MarkupCleanerTester> (MarkupCleanerTester.test_unsupported_entities_are_raw));
     Test.add_data_func ("/Unit/MarkupCleaner/NumericEntitiesArePreserved", Fixture.create<MarkupCleanerTester> (MarkupCleanerTester.test_num_entities_are_preserved));
 
-    var ml = new MainLoop ();
-    ScopeRegistry.instance ().find_scopes.begin ((obj, res) =>
+    Test.add_data_func ("/Unit/HomeScopeInstance/PhoneFilters", Fixture.create<HomeScopeInstanceTester> (HomeScopeInstanceTester.test_phone_filters));
+    Test.add_data_func ("/Unit/HomeScopeInstance/DesktopFilters", Fixture.create<HomeScopeInstanceTester> (HomeScopeInstanceTester.test_desktop_filters));
+
+    Environment.set_variable ("SMART_SCOPES_SERVER", "http://127.0.0.1:%d".printf (fake_server.server_port), true);
+
+    HomeScope.discover_scopes_sync ();
+
+    app = Extras.dbus_own_name (HOME_SCOPE_DBUS_NAME, () =>
     {
-      ScopeRegistry.instance ().find_scopes.end (res);
-      kw.rebuild ();
-      ml.quit ();
+      scope = new HomeScope ();
     });
-    ml.run ();
-    MetaScopeRegistry.instance ().update (ScopeRegistry.instance (), null);
+    kw.rebuild ();
 
     Test.run ();
 
+    fake_server.stop ();
+
     return 0;
   }
-  
-  public static bool run_with_timeout (MainLoop ml, uint timeout_ms = 5000)
+
+  class HomeScopeInstanceTester: Object, Fixture
   {
-    bool timeout_reached = false;
-    var t_id = Timeout.add (timeout_ms, () =>
-    {
-      timeout_reached = true;
-      debug ("Timeout reached");
-      ml.quit ();
-      return false;
-    });
-
-    ml.run ();
-
-    if (!timeout_reached) Source.remove (t_id);
-
-    return !timeout_reached;
+    ScopeProxy? proxy = null;
+    string channel_id;
+
+    private void setup ()
+    {
+      if (!scope.smart_scopes_ready) {
+        var ml = new MainLoop ();
+        scope.notify["smart_scopes_ready"].connect(() => { ml.quit (); });
+        run_with_timeout (ml, 8);
+      }
+
+      proxy = acquire_test_proxy (HOME_SCOPE_DBUS_NAME, HOME_SCOPE_DBUS_PATH);
+      channel_id = open_channel (proxy, ChannelType.GLOBAL, null);
+      assert (channel_id != null);
+    }
+
+    private void teardown ()
+    {
+      proxy.close_channel (channel_id, null);
+      proxy = null;
+    }
+
+    internal void test_phone_filters ()
+    {
+      // ignore warnings
+      Test.log_set_fatal_handler (() => { return false; });
+
+      bool got_filters_update = false;
+
+      proxy.filter_settings_changed.connect ((chid, filter_rows) => {
+          got_filters_update = true;
+      });      
+      
+      var hints = new HashTable<string, Variant> (str_hash, str_equal);
+      hints["form-factor"] = "phone";
+      perform_search (proxy, channel_id, "metallica", hints, null);
+
+      assert (got_filters_update == false); // we expect no filter updates on the phone
+    }
+
+    internal void test_desktop_filters ()
+    {
+      // ignore warnings
+      Test.log_set_fatal_handler (() => { return false; });
+
+      int got_filters_update = 0;
+
+      Variant? filters = null;
+      proxy.filter_settings_changed.connect ((chid, filter_rows) => {
+          got_filters_update += 1;
+          // note: there are 2 filter updates during search, but we care about the final value of filters
+          filters = filter_rows;
+      });
+    
+      assert (channel_id != null);
+      assert (got_filters_update == 0);
+
+      wait_for_synchronization (proxy.filters_model);
+      assert (proxy.filters_model.get_n_rows () == 2);
+      
+      // verify all filter options are initially inactive
+      int option_count = 0;
+      for (var iter = proxy.filters_model.get_first_iter (); iter != proxy.filters_model.get_last_iter (); iter = proxy.filters_model.next (iter))
+      {
+        var opts = proxy.filters_model.get_row (iter)[4].lookup_value ("options", null);
+        for (int i = 0; i<opts.n_children (); i++)
+        {
+          option_count += 1;
+          var opt = opts.get_child_value(i);
+          assert (opt.get_child_value (3).get_boolean () == false);
+        }
+      }
+
+      assert (option_count == 17);
+
+      var hints = new HashTable<string, Variant> (str_hash, str_equal);
+      hints["form-factor"] = "desktop";
+      perform_search (proxy, channel_id, "iron maiden", hints, null);
+
+      assert (got_filters_update == 2);
+      assert (filters.n_children() == 2); // two filters ('sources' and 'categories')
+      var src_filter = filters.get_child_value(0);
+      var cat_filter = filters.get_child_value(1);
+      
+      // verify 'sources' filter
+      var opts = src_filter.get_child_value (4).lookup_value ("options", null);
+      var option_flags = new HashTable<string, bool>(str_hash, str_equal);
+
+      for (int i = 0; i<opts.n_children (); i++) // create scopeid -> enabled flag lookup for sources filter
+      {
+          var opt = opts.get_child_value(i);
+          option_flags[opt.get_child_value (0).get_string ()] = opt.get_child_value (3).get_boolean ();
+      }
+
+      assert (option_flags["reference-stackexchange.scope"] == false);
+      assert (option_flags["reference-dictionary.scope"] == false);
+      assert (option_flags["reference-themoviedb.scope"] == false);
+      assert (option_flags["masterscope_b-subscope1.scope"] == false);
+      assert (option_flags["masterscope_b-subscope2.scope"] == false);
+      assert (option_flags["masterscope_a-subscope1.scope"] == false);
+      assert (option_flags["masterscope_a-subscope2.scope"] == false);
+      assert (option_flags["more_suggestions-amazon.scope"] == false);
+      assert (option_flags["more_suggestions-etsy.scope"] == false);
+      assert (option_flags["more_suggestions-ebay.scope"] == false);
+      assert (option_flags["more_suggestions-skimlinks.scope"] == false);
+      
+      assert (option_flags["more_suggestions-u1ms.scope"] == true);
+      assert (option_flags["reference-wikipedia.scope"] == true);
+
+      // verify 'categories' filter
+      opts = cat_filter.get_child_value (4).lookup_value ("options", null);
+      option_flags = new HashTable<string, bool>(str_hash, str_equal);
+
+      for (int i = 0; i<opts.n_children (); i++) // create scopeid -> enabled flag lookup for categories filter
+      {
+          var opt = opts.get_child_value(i);
+          option_flags[opt.get_child_value (0).get_string ()] = opt.get_child_value (3).get_boolean ();
+      }
+      
+      assert (option_flags["masterscope_a.scope"] == false);
+      assert (option_flags["masterscope_b.scope"] == false);
+
+      assert (option_flags["more_suggestions.scope"] == true);
+      assert (option_flags["reference.scope"] == true);
+    }
   }
 
   class HomeScopeSearchTester: Object, Fixture
@@ -178,7 +320,7 @@
       // scope registry is a singleton and is initialized in main on start
       var registry = ScopeRegistry.instance ();
       var scopes = registry.flatten ();
-      assert (scopes.size == 6);
+      assert (scopes.size == 8);
       assert (scopes.contains ("masterscope_a.scope"));
       assert (scopes.contains ("masterscope_b.scope"));
       assert (scopes.contains ("masterscope_a-subscope1.scope"));
@@ -189,7 +331,7 @@
 
     internal void test_keyword_search ()
     {
-      assert (kw.num_of_mappings == 8);
+      assert (kw.num_of_mappings == 9);
       string new_search_string;
 
       assert (kw.process_query ("abcd: foobar", out new_search_string) == null); //unknown keyword, leave query as is
@@ -652,6 +794,7 @@
     internal void test_smart_scopes_parse ()
     {
       CategoryManager.instance ().register ("more_suggestions.scope");
+      CategoryManager.instance ().register ("reference.scope");
 
       int row_count = 0;
       int recommend_count = 0;
@@ -783,6 +926,8 @@
     internal void test_smart_scopes_on_chunk_data ()
     {
       CategoryManager.instance ().register ("more_suggestions.scope");
+      CategoryManager.instance ().register ("reference.scope");
+
       int recommend_count = 0;
       int result_count = 0;
 
@@ -1003,7 +1148,7 @@
       installed.add ("scope2.scope");
       installed.add ("scope4.scope");
 
-      var clinfo = ClientScopesInfo.from_file (Config.TOPSRCDIR + "/tests/unit/data/client-scopes.json", installed);
+      var clinfo = ClientScopesInfo.from_file (Config.TOPSRCDIR + "/tests/unit/data/unity/client-scopes.json", installed);
       var added = clinfo.get_added_scopes ();
       var removed = clinfo.get_removed_scopes ();
 
@@ -1062,59 +1207,12 @@
 
   class SmartScopesInterfaceTester: Object, Fixture
   {
-    private Rand rand = new Rand ();
-    private Pid server_pid;
-    private int server_port;
-
-    private void start_fake_server () throws SpawnError
-    {
-      server_port = rand.int_range (1024, 9000); 
-
-      Process.spawn_async (null, {Config.TOPSRCDIR + "/tests/fake-server/fake-sss-server.py",
-                  "--scopes", Config.TOPSRCDIR + "/tests/fake-server/samples/remote-scopes.txt",
-                  "--search", Config.TESTRUNDATADIR + "/search.dump",
-                  "--feedback", Config.TESTRUNDATADIR + "/feedback.dump",
-                  "--requests", "2",
-                  "--timeout", "5",
-                  "--port", server_port.to_string (),
-                  Config.TOPSRCDIR + "/tests/unit/data/search_results1.txt"},
-                  null, 0, null, out server_pid);
-
-      Socket socket = new Socket (SocketFamily.IPV4, SocketType.STREAM, SocketProtocol.TCP);
-      assert (socket != null);
-
-      InetAddress addr = new InetAddress.from_bytes ({127, 0, 0, 1}, SocketFamily.IPV4);
-      InetSocketAddress server_addr = new InetSocketAddress (addr, (uint16)server_port);
-
-      bool conn = false;
-      int retry = 5;
-      while (!conn && retry > 0)
-      {
-        try
-        {
-          conn = socket.connect (server_addr);
-        }
-        catch (Error e) {}
-        if (!conn)
-          Thread.usleep (1*1000000); // sleep for 1 second
-        --retry;
-      }
-      Process.close_pid (server_pid);
-    }
-
-    private void setup ()
-    {
-      start_fake_server ();
-    }
-
-    private void teardown ()
-    {
-      Posix.kill (server_pid, Posix.SIGTERM);
-    }
-
     internal void test_smart_scopes_client_iface_search ()
     {
-      Environment.set_variable ("SMART_SCOPES_SERVER", "http://127.0.0.1:%d".printf (server_port), true);
+      // ignore warnings
+      Test.log_set_fatal_handler (() => { return false; });
+
+      Environment.set_variable ("SMART_SCOPES_SERVER", "http://127.0.0.1:%d".printf (fake_server.server_port), true);
       var pinfo = new SmartScopes.PlatformInfo.with_data ("1304", "EN", {"scope1"}, {"scope3"});
       var client = new SmartScopes.SmartScopesClient (pinfo);
       var session_id = "5d06cc10-751b-11e2-87e0-fb468b0a185a";
@@ -1159,7 +1257,10 @@
 
     internal void test_smart_scopes_client_iface_metrics ()
     {
-      Environment.set_variable ("SMART_SCOPES_SERVER", "http://127.0.0.1:%d".printf (server_port), true);
+      // ignore warnings
+      Test.log_set_fatal_handler (() => { return false; });
+
+      Environment.set_variable ("SMART_SCOPES_SERVER", "http://127.0.0.1:%d".printf (fake_server.server_port), true);
       var pinfo = new SmartScopes.PlatformInfo.with_data ("1304", "EN", {"scope1"}, {"scope3"});
       var client = new SmartScopes.SmartScopesClient (pinfo);
       var session_id = "5d06cc10-751b-11e2-87e0-fb468b0a185a";
@@ -1245,6 +1346,9 @@
 
     internal void test_smart_scopes_client_iface_error ()
     {
+      // ignore warnings
+      Test.log_set_fatal_handler (() => { return false; });
+
       Environment.set_variable ("SMART_SCOPES_SERVER", "http://127.0.0.1:9999", true); // non-existing server (connection failure)
       var pinfo = new SmartScopes.PlatformInfo.with_data ("1304", "EN", {"scope1"}, {"scope3"});
       var client = new SmartScopes.SmartScopesClient (pinfo);
 
=== added file 'tests/unit/test-utils.vala'
--- tests/unit/test-utils.vala	1970-01-01 00:00:00 +0000
+++ tests/unit/test-utils.vala	2013-10-01 10:06:09 +0000
@@ -0,0 +1,194 @@
+/*
+ * Copyright (C) 2011-2013 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by Michal Hruby <[e-mail]>
+ *
+ */
+  
+using Unity.Protocol;
+
+public static bool run_with_timeout (MainLoop ml, uint timeout_ms = 5000)
+{
+  bool timeout_reached = false;
+  var t_id = Timeout.add (timeout_ms, () =>
+  {
+    timeout_reached = true;
+    debug ("Timeout reached");
+    ml.quit ();
+    return false;
+  });
+  
+  ml.run ();
+  
+  if (!timeout_reached) Source.remove (t_id);
+  
+  return !timeout_reached;
+}
+
+/* A bit of magic to get proper-ish fixture support */
+public interface Fixture : Object
+{
+  class DelegateWrapper
+  {
+    TestDataFunc func;
+    public DelegateWrapper (owned TestDataFunc f) { func = (owned) f; }
+  }
+  
+  public virtual void setup () {}
+  public virtual void teardown () {}
+  
+  [CCode (has_target = false)]
+  public delegate void Callback<T> (T ptr);
+  
+  private static List<DelegateWrapper> _tests;
+  
+  public static unowned TestDataFunc create<F> (Callback<void*> cb)
+  requires (typeof (F).is_a (typeof (Fixture)))
+  {
+    TestDataFunc functor = () =>
+    {
+      var type = typeof (F);
+      var instance = Object.new (type) as Fixture;
+      instance.setup ();
+      cb (instance);
+      instance.teardown ();
+    };
+    unowned TestDataFunc copy = functor;
+    _tests.append (new DelegateWrapper ((owned) functor));
+    return copy;
+  }
+  public static unowned TestDataFunc create_static<F> (Callback<F> cb)
+  {
+    return create<F> ((Callback<void*>) cb);
+  }
+}
+
+// this will auto-disconnect signals when it goes out of scope
+public class SignalWrapper
+{
+  unowned Object obj;
+  ulong sig_id;
+  
+  public SignalWrapper (Object o, ulong signal_id)
+  {
+    obj = o;
+    sig_id = signal_id;
+  }
+
+  ~SignalWrapper ()
+  {
+    SignalHandler.disconnect (obj, sig_id);
+  }
+}
+
+public static ScopeProxy? acquire_test_proxy (string name, string path)
+{
+  var ml = new MainLoop ();
+  ScopeProxy? proxy = null;
+  ScopeProxy.new_from_dbus.begin (name, path, null, (obj, res) =>
+  {
+    try
+    {
+      proxy = ScopeProxy.new_from_dbus.end (res);
+    }
+    catch (Error e) {}
+    ml.quit ();
+  });
+  assert (run_with_timeout (ml));
+  return proxy;
+}
+
+public static void wait_for_synchronization (Dee.Model model)
+{
+  var shared_model = model as Dee.SharedModel;
+  if (shared_model == null) return;
+  
+  if (shared_model.is_synchronized ()) return;
+  SignalWrapper[] signals = {};
+  var ml = new MainLoop ();
+  
+  signals += new SignalWrapper (shared_model, 
+                                shared_model.notify["synchronized"].connect (() =>
+                                {
+                                  ml.quit ();
+                                }));
+  
+  run_with_timeout (ml);
+}
+
+public static string open_channel (ScopeProxy proxy,
+                                   ChannelType channel_type,
+                                   out Dee.SerializableModel model,
+                                   bool wait_for_sync = false,
+                                   ChannelFlags flags = 0)
+{
+  string? channel_id = null;
+  Dee.Model? real_model = null;
+  var ml = new MainLoop ();
+  /* Need to use PRIVATE channel, cause standard SharedModel won't
+   * synchronize properly when trying to connect to the model
+   * from the same process (/bus address) */
+  proxy.open_channel.begin (channel_type,
+                            flags | ChannelFlags.PRIVATE,
+                            null,
+                            (obj, res) =>
+                            {
+                              try
+                              {
+                                channel_id = proxy.open_channel.end (res, out real_model);
+                                if (wait_for_sync)
+                                {
+                                  wait_for_synchronization (real_model);
+                                }
+                                ml.quit ();
+                              }
+                              catch (Error err)
+                              {
+                                ml.quit ();
+                              }
+                            });
+  
+  assert (run_with_timeout (ml));
+  assert (channel_id != null);
+  model = real_model as Dee.SerializableModel;
+  return channel_id;
+}
+
+public static HashTable<string, Variant> perform_search (
+  ScopeProxy proxy, string channel_id, string query,
+  HashTable<string, Variant>? hints = null,
+  Dee.SerializableModel? model = null)
+{
+  var ml = new MainLoop ();
+  HashTable<string, Variant>? reply_dict = null;
+  proxy.search.begin (channel_id, query,
+                      hints ?? new HashTable<string, Variant> (null, null),
+                      null,
+                      (obj, res) =>
+                      {
+                        try
+                        {
+                          reply_dict = proxy.search.end (res);
+                        }
+                        catch (Error err) {}
+                        ml.quit ();
+                      });
+  
+  bool got_search_signal = false;
+  
+  assert (run_with_timeout (ml, 10000));
+  assert (reply_dict != null);
+  return reply_dict;
+}