webbrowser-app

Merge lp://staging/~osomon/webbrowser-app/systemwide-search-engines into lp://staging/webbrowser-app

systemwide-search-engines
Merge into trunk

Proposed by Olivier Tilloy on 2015-04-29

Status:

Merged

Approved by:

Olivier Tilloy on 2015-05-15

Approved revision:

997

Merged at revision:

1012

Proposed branch:

lp://staging/~osomon/webbrowser-app/systemwide-search-engines

Merge into:

lp://staging/webbrowser-app

Diff against target:

1134 lines (+752/-69)

22 files modified

debian/control (+1/-0)
src/app/config.h.in (+0/-9)
src/app/webbrowser/Browser.qml (+1/-0)
src/app/webbrowser/CMakeLists.txt (+4/-0)
src/app/webbrowser/SearchEngines.qml (+91/-0)
src/app/webbrowser/SettingsPage.qml (+13/-10)
src/app/webbrowser/searchengine.cpp (+91/-49)
src/app/webbrowser/searchengine.h (+13/-0)
src/app/webbrowser/searchengines/bing.xml (+6/-0)
src/app/webbrowser/searchengines/duckduckgo.xml (+6/-0)
src/app/webbrowser/searchengines/google.xml (+6/-0)
src/app/webbrowser/searchengines/wikipedia.xml (+6/-0)
src/app/webbrowser/searchengines/yahoo.xml (+6/-0)
src/app/webbrowser/webbrowser-app.cpp (+7/-0)
tests/autopilot/webbrowser_app/emulators/browser.py (+7/-0)
tests/autopilot/webbrowser_app/tests/test_settings.py (+43/-1)
tests/unittests/CMakeLists.txt (+1/-0)
tests/unittests/qml/CMakeLists.txt (+1/-0)
tests/unittests/qml/tst_QmlTests.cpp (+81/-0)
tests/unittests/qml/tst_SearchEngines.qml (+117/-0)
tests/unittests/search-engine/CMakeLists.txt (+9/-0)
tests/unittests/search-engine/tst_SearchEngineTests.cpp (+242/-0)

To merge this branch:

bzr merge lp://staging/~osomon/webbrowser-app/systemwide-search-engines

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-05-14
Ugo Riboni (community)		2015-04-29	Needs Fixing on 2015-05-06
Review via email: mp+257830@code.staging.launchpad.net

Commit message

Look for custom search engines description files in several locations.
This adds a build dependency on qml-module-qt-labs-folderlistmodel, to run unit tests at package construction time.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-30:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1712/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Ugo Riboni (uriboni) wrote on 2015-05-06:

Running the new AP tests gives this error:

Traceback (most recent call last):
  File "/home/nerochiaro/projects/phone/webbrowser-app/tests/autopilot/webbrowser_app/tests/test_settings.py", line 45, in test_open_close_searchengine_page
    self.assertThat(old_engine, NotEquals(""))
  File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 423, in assertThat
    raise mismatch_error
testtools.matchers._impl.MismatchError: '' == ''

More info on my environment:
I have one file in ~/.local/share/webbrowser-app/searchengines called bing.xml and completely empty
I built the branch in-tree and I am running the tests from tests/autopilot

This seems to indicate that the browser is reading the files from ~/.local/share/webbrowser-app which should not happen during tests.

First verify why this makes the tests fail. I don't see why it should, so I suspect a bug somewhere.

Second, we are testing by doing things in the current user directory. This is not safe. We should add a test mode command line switch to the app which calls QStandardPaths::setTestModeEnabled(true), so that QT will return special test paths for all standard paths. Then run our tests in this mode.

Once that is done we should ideally set up more AP tests to verify the following features:
- test that the names and descriptions of the engines correspond to what is read from the files
- test overriding an engine by placing a file of the same name, and verifying that the search url, description and name are correctly overriden
- test overriding an engine by placing an empty file of the same name, then verifying that the engine is removed
- verify a few of the above by manipulating the files in the list while app is running and verifying that the settings page picks these up the next time it is opened.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2015-05-08:

> Running the new AP tests gives this error:
>
> Traceback (most recent call last):
> File "/home/nerochiaro/projects/phone/webbrowser-
> app/tests/autopilot/webbrowser_app/tests/test_settings.py", line 45, in
> test_open_close_searchengine_page
> self.assertThat(old_engine, NotEquals(""))
> File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 423, in
> assertThat
> raise mismatch_error
> testtools.matchers._impl.MismatchError: '' == ''
>
> More info on my environment:
> I have one file in ~/.local/share/webbrowser-app/searchengines called bing.xml
> and completely empty
> I built the branch in-tree and I am running the tests from tests/autopilot

This test passes here, tests ran in the same conditions. Can you reliably reproduce the failure?

> This seems to indicate that the browser is reading the files from
> ~/.local/share/webbrowser-app which should not happen during tests.
>
> First verify why this makes the tests fail. I don't see why it should, so I
> suspect a bug somewhere.
>
> Second, we are testing by doing things in the current user directory. This is
> not safe. We should add a test mode command line switch to the app which calls
> QStandardPaths::setTestModeEnabled(true), so that QT will return special test
> paths for all standard paths. Then run our tests in this mode.

This will be addressed by https://code.launchpad.net/~osomon/webbrowser-app/autopilot-temp-profile/+merge/257490, although the implementation differs from your suggestion, but achieves the same result. It’s in a silo, awaiting QA verification for landing.

> Once that is done we should ideally set up more AP tests to verify the
> following features:
> - test that the names and descriptions of the engines correspond to what is
> read from the files

This is already unit-tested.

> - test overriding an engine by placing a file of the same name, and verifying
> that the search url, description and name are correctly overriden
> - test overriding an engine by placing an empty file of the same name, then
> verifying that the engine is removed

I’ll add unit tests for the above two suggestions, thanks.

> - verify a few of the above by manipulating the files in the list while app is
> running and verifying that the settings page picks these up the next time it
> is opened.

Although a valid test, that sounds a bit overkill for an autopilot test. I’ll see if I can write a QML test for it though.

> Running the new AP tests gives this error:
> 
> Traceback (most recent call last):
>   File "/home/nerochiaro/projects/phone/webbrowser-
> app/tests/autopilot/webbrowser_app/tests/test_settings.py", line 45, in
> test_open_close_searchengine_page
>     self.assertThat(old_engine, NotEquals(""))
>   File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 423, in
> assertThat
>     raise mismatch_error
> testtools.matchers._impl.MismatchError: '' == ''
> 
> More info on my environment:
> I have one file in ~/.local/share/webbrowser-app/searchengines called bing.xml
> and completely empty
> I built the branch in-tree and I am running the tests from tests/autopilot

This test passes here, tests ran in the same conditions. Can you reliably reproduce the failure?

> This seems to indicate that the browser is reading the files from
> ~/.local/share/webbrowser-app which should not happen during tests.
> 
> First verify why this makes the tests fail. I don't see why it should, so I
> suspect a bug somewhere.
> 
> Second, we are testing by doing things in the current user directory. This is
> not safe. We should add a test mode command line switch to the app which calls
> QStandardPaths::setTestModeEnabled(true), so that QT will return special test
> paths for all standard paths. Then run our tests in this mode.

> Once that is done we should ideally set up more AP tests to verify the
> following features:
> - test that the names and descriptions of the engines correspond to what is
> read from the files

This is already unit-tested.

I’ll add unit tests for the above two suggestions, thanks.

> - verify a few of the above by manipulating the files in the list while app is
> running and verifying that the settings page picks these up the next time it
> is opened.

Although a valid test, that sounds a bit overkill for an autopilot test. I’ll see if I can write a QML test for it though.

lp://staging/~osomon/webbrowser-app/systemwide-search-engines updated on 2015-05-08

990. By Olivier Tilloy on 2015-05-08: Add a couple of unit tests for SearchEngine.
991. By Olivier Tilloy on 2015-05-08: Add QML tests for the SearchEngines component.
992. By Olivier Tilloy on 2015-05-08: Improve the SearchEngines component to update the list of engines whenever a description file is added/removed.
993. By Olivier Tilloy on 2015-05-08: Remove an unused test helper.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-08:

FAILED: Continuous integration, rev:993
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1736/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2693/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/493/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/493/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/493/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2691/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1736/rebuild

review: Needs Fixing (continuous-integration)

lp://staging/~osomon/webbrowser-app/systemwide-search-engines updated on 2015-05-08

994. By Olivier Tilloy on 2015-05-08: Add missing build dependency.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-08:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1737/rebuild

review: Needs Fixing (continuous-integration)

lp://staging/~osomon/webbrowser-app/systemwide-search-engines updated on 2015-05-14

995. By Olivier Tilloy on 2015-05-14: Merge the latest changes from trunk and resolve a bunch of conflicts.
996. By Olivier Tilloy on 2015-05-14: Also test the suggestionsUrlTemplate property in the unit tests.
997. By Olivier Tilloy on 2015-05-14: Cosmetics.