Merge lp://qastaging/~knitzsche/scope-aggregator/gtest into lp://qastaging/scope-aggregator

Proposed by Kyle Nitzsche
Status: Merged
Merged at revision: 173
Proposed branch: lp://qastaging/~knitzsche/scope-aggregator/gtest
Merge into: lp://qastaging/scope-aggregator
Diff against target: 6141 lines (+4060/-1010)
33 files modified
.bzrignore (+5/-0)
CMakeLists.txt (+1/-0)
gtests/CMakeLists.txt (+78/-0)
gtests/MockRegistry.h (+167/-0)
gtests/TypedScopeFixture.h (+136/-0)
gtests/main.cpp (+7/-0)
gtests/query_test.cpp (+64/-0)
gtests/scope_directory/child_scopes.json (+330/-0)
gtests/scope_directory/cs1.json (+346/-0)
gtests/scope_directory/fallback_1.json (+70/-0)
gtests/scope_directory/find_test_1.json (+28/-0)
gtests/scope_directory/find_test_2.json (+28/-0)
gtests/scope_directory/find_test_3.json (+28/-0)
gtests/scope_directory/find_test_4.json (+28/-0)
gtests/scope_directory/find_test_5.json (+38/-0)
gtests/scope_directory/find_test_6.json (+38/-0)
gtests/scope_directory/find_test_7.json (+37/-0)
gtests/scope_directory/find_test_8.json (+37/-0)
gtests/scope_directory/find_test_full.json (+102/-0)
gtests/scope_directory/hints.json (+147/-0)
gtests/scope_directory/hints_local.json (+136/-0)
gtests/scope_directory/hints_not_using.json (+3/-0)
gtests/scope_directory/hints_remote.json (+144/-0)
gtests/scope_directory/no_departments.json (+214/-0)
gtests/scope_test.cpp (+645/-0)
include/client.h (+42/-0)
include/query.h (+22/-10)
include/scope.h (+15/-8)
src/CMakeLists.txt (+1/-1)
src/client.cpp (+94/-0)
src/query.cpp (+100/-810)
src/scope.cpp (+59/-38)
src/utils.cpp (+870/-143)
To merge this branch: bzr merge lp://qastaging/~knitzsche/scope-aggregator/gtest
Reviewer Review Type Date Requested Status
Zhang Enwei (community) Approve
Paweł Stołowski Pending
Gary.Wang Pending
Review via email: mp+303464@code.qastaging.launchpad.net

Description of the change

Here is an initial implementation of google test/mock for scope-aggregator including (at this writing) 20 tests.

Lots of core parts are tested including loading numerous test child scope json files and verifying the proper results (departments are registered if declared, cardinality is correct, etc.). A MockRegistry pretends numerous child scopes are installed and enabled with various scope IDs and responsive to various keywords, and the agg scope is tested to ensure the correct number of child scopes are found for various combinations of declared scopes, keywords and categories. Various hints json combinations are tested (local, remote, remote with no hints scope instaled, hints stops displaying after being dismissed, etc.). Fallback functionality is also tested. Naturally, more can be added.

I also did some refactoring, some of which was needed to implement the tests. This could be extended.

A few notes:

* I refactored Query::handle_child_scope_results() moving most of the code into three new methods for readability and etc.

* Currently the tests do not run automatically on build [1]. This is only because it does not work in a click chroot (because they cross build) and click-buddy uses a click chroot. We do not need to build with click-buddy since scope-aggregrator itself is not installed as a click. (I can build in an armhf pbuilder chroot and the tests run and pass, but this is very slow). I am open to suggestions. When developing scope-agg I do thist o build and run tests:
    mkdir build && cd build
    cmake PATH-TO-SOURCETREE
    make ** ./gtests/tests

I still build the lib...so in an armhf click chroot (via click-buddy) to use in aggregators, only for convenience. If we drop this build option we can run tests on every build.

There is a commented out cmake snippet to auto run tests post build in gtests/CMakeLists.txt.

* I created a Client class and moved some code into it, for example the useful qstr() and sstr() methods. Now scope and query have a client object for use.

* I deleted the tests/ dir which was used for autopilot, but I don't think scope-aggregator needs autopilot.

* I merged in trunk on aug 9 whihch picks up the latest commit (Enwei's rev 171.) So it is up to date.

* I tested on Today, News, Nearby and Photos and all seemed to work.

* I was not able to actually execute the query's run() method in a test: I get a message saying the agg_scope->child_scopes() method cannot be run from a constructor. Not sure why that happens. But even without this, I do create a Query object and test various methods so we have reasonable first pass test coverage.

[1]

[100%] Built target gtest_main
[==========] Running 20 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 20 tests from TestAggScope
[ RUN ] TestAggScope.scopeId
[ OK ] TestAggScope.scopeId (1 ms)
[ RUN ] TestAggScope.verify_chlld_scopes_json
[ OK ] TestAggScope.verify_chlld_scopes_json (0 ms)
[ RUN ] TestAggScope.verify_no_departments
[ OK ] TestAggScope.verify_no_departments (1 ms)
[ RUN ] TestAggScope.verify_cardinality_methods
[ OK ] TestAggScope.verify_cardinality_methods (0 ms)
[ RUN ] TestAggScope.get_declared_chlld_scopes
[ OK ] TestAggScope.get_declared_chlld_scopes (0 ms)
[ RUN ] TestAggScope.get_declared_keywords
[ OK ] TestAggScope.get_declared_keywords (0 ms)
[ RUN ] TestAggScope.find_child_scopes_1_no_valid_declared
[ OK ] TestAggScope.find_child_scopes_1_no_valid_declared (1 ms)
[ RUN ] TestAggScope.find_child_scopes_2_one_valid_declared
[ OK ] TestAggScope.find_child_scopes_2_one_valid_declared (0 ms)
[ RUN ] TestAggScope.find_child_scopes_3_no_valid_keywords
[ OK ] TestAggScope.find_child_scopes_3_no_valid_keywords (1 ms)
[ RUN ] TestAggScope.find_child_scopes_4_one_valid_keyword
[ OK ] TestAggScope.find_child_scopes_4_one_valid_keyword (0 ms)
[ RUN ] TestAggScope.find_child_scopes_5_no_valid_category_scope
[ OK ] TestAggScope.find_child_scopes_5_no_valid_category_scope (1 ms)
[ RUN ] TestAggScope.find_child_scopes_6_one_valid_category_scope
[ OK ] TestAggScope.find_child_scopes_6_one_valid_category_scope (0 ms)
[ RUN ] TestAggScope.find_child_scopes_7_no_valid_category_keyword
[ OK ] TestAggScope.find_child_scopes_7_no_valid_category_keyword (1 ms)
[ RUN ] TestAggScope.find_child_scopes_8_one_valid_category_keyword
[ OK ] TestAggScope.find_child_scopes_8_one_valid_category_keyword (1 ms)
[ RUN ] TestAggScope.find_child_scopes_full
[ OK ] TestAggScope.find_child_scopes_full (0 ms)
[ RUN ] TestAggScope.fallback
[ OK ] TestAggScope.fallback (0 ms)
[ RUN ] TestAggScope.make_scope
[ OK ] TestAggScope.make_scope (1 ms)
[ RUN ] TestAggScope.hints_not_using
[ OK ] TestAggScope.hints_not_using (0 ms)
[ RUN ] TestAggScope.hints_local
"com.canonical.scopes.fitbit_fitbit: OA scope is NOT REGISTERED, skipping for HINTS: %2"
[ OK ] TestAggScope.hints_local (3 ms)
[ RUN ] TestAggScope.hints_remote
[ OK ] TestAggScope.hints_remote (1 ms)
[----------] 20 tests from TestAggScope (12 ms total)

[----------] Global test environment tear-down
[==========] 20 tests from 1 test case ran. (12 ms total)
[ PASSED ] 20 tests.

To post a comment you must log in.
Revision history for this message
Zhang Enwei (zhangew401) wrote :

I verified the gtest and scope aggregator library on news-china, china-dashboard, baidu.

review: Approve
Revision history for this message
Kyle Nitzsche (knitzsche) wrote :

Almost a month: one approve, no other responses. I'll merge to trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches