Merge into trunk : add-integration-tests-for-webappcontainer : Code : webbrowser-app

Status:	Merged
Merged at revision:	492
Proposed branch:	lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer
Merge into:	lp:webbrowser-app
Diff against target:	367 lines (+273/-6) 11 files modified debian/control (+12/-0) debian/webapp-container-autopilot.install (+1/-0) src/app/webcontainer/WebApp.qml (+2/-0) src/app/webcontainer/webapp-container.cpp (+3/-1) src/app/webcontainer/webapp-container.qml (+2/-0) tests/autopilot/CMakeLists.txt (+3/-5) tests/autopilot/webapp_container/__init__.py (+16/-0) tests/autopilot/webapp_container/tests/__init__.py (+82/-0) tests/autopilot/webapp_container/tests/fake_servers.py (+68/-0) tests/autopilot/webapp_container/tests/test_app_launch.py (+36/-0) tests/autopilot/webapp_container/tests/test_chrome_setup.py (+48/-0)
To merge this branch:	bzr merge lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2014-04-11
Olivier Tilloy		2013-12-16	Approve on 2014-04-09
Leo Arias (community)	code review		Needs Fixing on 2014-01-31
Review via email: mp+199089@code.launchpad.net

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-12-16:

#

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

review: Approve (continuous-integration)

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-01-07

426. By Alexandre Abreu on 2013-12-16

Restrict url webapps pattern replacement depending on which part is being affected. Fixes: https://bugs.launchpad.net/bugs/1240685.

Approved by PS Jenkins bot, Olivier Tilloy.

427. By Olivier Tilloy on 2013-12-16

Ensure the test server is up and responding before starting the tests,
and use standard logging everywhere.

Approved by PS Jenkins bot.

428. By Alexandre Abreu on 2013-12-18

Fix back forward hud actions.

Approved by Olivier Tilloy, PS Jenkins bot.

429. By Launchpad Translations on behalf of phablet-team on 2013-12-24

Launchpad automatic translations update.

430. By Launchpad Translations on behalf of phablet-team on 2014-01-05

Launchpad automatic translations update.

431. By Launchpad Translations on behalf of phablet-team on 2014-01-07

Launchpad automatic translations update.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-07:

#

When listing the available autopilot tests for webapp_container locally from my development branch (and without the container installed system-wide), I’m getting the following error:

$ autopilot list webapp_container
Loading tests from: /home/osomon/dev/phablet/browser/webbrowser-app/tests/autopilot

Failed to import test module: webapp_container.tests.test_chromeSetup
Traceback (most recent call last):
  File "/usr/lib/python2.7/unittest/loader.py", line 254, in _find_tests
    module = self._get_module_from_name(name)
  File "/usr/lib/python2.7/unittest/loader.py", line 232, in _get_module_from_name
    __import__(name)
  File "/home/osomon/dev/phablet/browser/webbrowser-app/tests/autopilot/webapp_container/tests/__init__.py", line 23, in <module>
    class WebappContainerTestCaseBase(AutopilotTestCase):
  File "/home/osomon/dev/phablet/browser/webbrowser-app/tests/autopilot/webapp_container/tests/__init__.py", line 26, in WebappContainerTestCaseBase
    ['which', 'webapp-container']).strip()
  File "/usr/lib/python2.7/subprocess.py", line 573, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command '['which', 'webapp-container']' returned non-zero exit status 1

The call to subprocess.check_output(…) should be surrounded with a try… except… block.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-07:

#

Minor nit:

16 +Description: Webapp container autopilot tests

should be:

Description: Ubuntu web applications container autopilot tests

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-07:

#

set(WEBBROWSER_APP_AUTOPILOT_DIR webbrowser_app)

install(DIRECTORY ${WEBBROWSER_APP_AUTOPILOT_DIR}
DESTINATION ${PYTHON_PACKAGE_DIR}
)

set(WEBCONTAINER_AUTOPILOT_DIR webapp_container)

install(DIRECTORY ${WEBCONTAINER_AUTOPILOT_DIR}
DESTINATION ${PYTHON_PACKAGE_DIR}
)

This could be simplified to:

    install(DIRECTORY webbrowser_app webapp_container
            DESTINATION ${PYTHON_PACKAGE_DIR}
    )

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-07:

#

142 +"""webapp_container autopilot tests and emulators - top level package."""

This one (tests/autopilot/webapp_container/emulators/__init__.py) is not a top-level package. The corresponding file for the browser app (tests/autopilot/webbrowser_app/emulators/__init__.py) is empty except for the copyright header, so this one should have exactly the same contents.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-07:

#

Running flake8 on tests/autopilot/webapp_container exhibits 46 errors/warnings. Can you please fix them?

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-07:

#

241 +class WebappContainerContentHttpServer(threading.Thread):

Can we re-use the test server already present in the tests for webbrowser-app, if it makes sense?

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-07:

#

For consistency with the existing tests, can you rename the test methods to use only lowercase and underscores to separate words, like so: "test_loads_with_url" instead of "test_loadsWithUrl".

review: Needs Fixing

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-01-07:

#

All done, besides reusing the test server since I don't want to add extra deps on this, and the commonalities at this point are slim imo,

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-07:

#

166 +except:

Unqualified excepts should be avoided as much as possible, they make it hard to spot problems. Please replace with:

except CalledProcessError:

171 + LOCAL_BROWSER_CONTAINER_PATH_NAME = "%s/%s" % (BASE_FILE_PATH,
172 + CONTAINER_EXEC_REL_PATH)

any reason for not doing this outside the class declaration?

review: Needs Fixing

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-01-07:

#

> 166 +except:
>
> Unqualified excepts should be avoided as much as possible, they make it hard
> to spot problems. Please replace with:
>
> except CalledProcessError:
>
>
> 171 + LOCAL_BROWSER_CONTAINER_PATH_NAME = "%s/%s" % (BASE_FILE_PATH,
> 172 +
> CONTAINER_EXEC_REL_PATH)
>
> any reason for not doing this outside the class declaration?

agree, ... done

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-01-07:

#

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

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-01-07:

#

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

review: Approve (continuous-integration)

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-01-09

432. By Olivier Tilloy on 2014-01-08: Bump version to 0.23 in preparation for the next major release.
433. By Launchpad Translations on behalf of phablet-team on 2014-01-09: Launchpad automatic translations update.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-09:

#

178 + self.pointer = Pointer(Mouse.create())

This fails on devices, where X11 is not available:

Traceback (most recent call last):
File "/usr/lib/python2.7/dist-packages/webapp_container/tests/test_appLaunch.py", line 19, in setUp
super(WebappContainerAppLaunchTestCase, self).setUp()
File "/usr/lib/python2.7/dist-packages/webapp_container/tests/__init__.py", line 121, in setUp
super(WebappContainerTestCaseWithLocalContentBase, self).setUp()
File "/usr/lib/python2.7/dist-packages/webapp_container/tests/__init__.py", line 38, in setUp
self.pointer = Pointer(Mouse.create())
File "/usr/lib/python2.7/dist-packages/autopilot/input/__init__.py", line 293, in create
"Cannot create a Mouse on devices where X11 is not available."
RuntimeError: Cannot create a Mouse on devices where X11 is not available.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-09:

#

180 + def tearDown(self):
181 + super(WebappContainerTestCaseBase, self).tearDown()

If all the method does is call its parent implementation, then it can be removed.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-09:

#

Note for when this MR lands: we will need to update the CI job to also run the webapp_container test suite (currently only the webbrowser_app one is run).

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-01-09:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-01-09:

#

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

review: Approve (continuous-integration)

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-01-09:

#

all done

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-01-09:

#

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

review: Needs Fixing (continuous-integration)

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-01-26

434. By Launchpad Translations on behalf of phablet-team on 2014-01-12

Launchpad automatic translations update.

435. By David Barth on 2014-01-13

Re-execute web application in the dedicated container (compatibility mode for legacy webapps).

Approved by PS Jenkins bot, Alexandre Abreu.

436. By PS Jenkins bot on 2014-01-17

Releasing 0.23+14.04.20140115.1-0ubuntu1 (revision 435 from lp:webbrowser-app).

Approved by PS Jenkins bot.

437. By Launchpad Translations on behalf of phablet-team on 2014-01-18

Launchpad automatic translations update.

438. By Launchpad Translations on behalf of phablet-team on 2014-01-19

Launchpad automatic translations update.

439. By Olivier Tilloy on 2014-01-20

Work around a bug (regression?) in the UITK by replacing the use of Page by a simple Item. Fixes: https://bugs.launchpad.net/bugs/1270848.

Approved by PS Jenkins bot, Alexandre Abreu, Oliver Grawert.

440. By PS Jenkins bot on 2014-01-20

Releasing 0.23+14.04.20140120-0ubuntu1 (revision 439 from lp:webbrowser-app).

Approved by PS Jenkins bot.

441. By Launchpad Translations on behalf of phablet-team on 2014-01-21

Launchpad automatic translations update.

442. By Ted Gould on 2014-01-21

Add URL Dispatcher configuration file.

Approved by PS Jenkins bot, Olivier Tilloy.

443. By Olivier Tilloy on 2014-01-21

Fix a memory leak when exiting the application.

Approved by Alexandre Abreu, PS Jenkins bot.

444. By Olivier Tilloy on 2014-01-22

Work around bug #1270848 in a different manner to avoid introducing another regression. Fixes: https://bugs.launchpad.net/bugs/1271436.

Approved by Łukasz Zemczak, PS Jenkins bot.

445. By PS Jenkins bot on 2014-01-22

Releasing 0.23+14.04.20140122-0ubuntu1 (revision 444 from lp:webbrowser-app).

Approved by PS Jenkins bot.

446. By Launchpad Translations on behalf of phablet-team on 2014-01-26

Launchpad automatic translations update.

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-01-27:

#

ping?

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-01-28:

#

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

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-01-28:

#

When running the tests on my device (with `phablet-test-run -o . webapp_container`), I’m getting 4 occurrences of the same failure:

<failure type="testtools.testresult.real._StringException">_StringException: Empty attachments:
process-stdout

process-return-code: {{{-6}}}
process-stderr: {{{
/usr/bin/webapp-container: invalid option -- 't'
/usr/bin/webapp-container: invalid option -- 'e'
/usr/bin/webapp-container: invalid option -- 's'
/usr/bin/webapp-container: invalid option -- 't'
/usr/bin/webapp-container: invalid option -- 'a'
/usr/bin/webapp-container: invalid option -- 'b'
/usr/bin/webapp-container: invalid option -- 'i'
/usr/bin/webapp-container: invalid option -- 'l'
/usr/bin/webapp-container: invalid option -- 'i'
/usr/bin/webapp-container: invalid option -- 't'
/usr/bin/webapp-container: invalid option -- 'y'
/usr/bin/webapp-container: unrecognized option '--webapp'
/usr/bin/webapp-container: unrecognized option '--enable-addressbar'
QUbuntu: Could not create application instance
}}}

Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/webapp_container/tests/test_chromeSetup.py", line 42, in test_enable_chrome_address_bar
    self.assertThat(self.get_webcontainer_proxy(), NotEquals(None))
  File "/usr/lib/python2.7/dist-packages/testtools/testcase.py", line 412, in assertThat
    raise MismatchError(matchee, matcher, mismatch, verbose)
MismatchError: None == None

</failure>

review: Needs Fixing

Revision history for this message

Leo Arias (elopio) wrote on 2014-01-31:

#

Please update the copyright years to include 2014.
The other files you touched it might need this too.

124 === added directory 'tests/autopilot/webapp_container/emulators'
125 === added file 'tests/autopilot/webapp_container/emulators/__init__.py'

Why do you need this empty emulators module?

173 + ARGS = []

ARGS is not a constant, so it should be lowercase.

205 +class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
232 +class WebappContainerContentHttpServer(threading.Thread):

To be consistent with what we are doing in other projects, I'd move this clases to a fake_servers.py module.

202 +HTTP_SERVER_PORT = 8383
236 + self.server = BaseHTTPServer.HTTPServer(("", port), RequestHandler)

If you pass 0 as the port, it will get a random free port, which is nice because the port is not released immediately after the server is shut down, so if tests are running too fast you might find that the port is occupied.

262 +++ tests/autopilot/webapp_container/tests/test_appLaunch.py 2014-01-09 17:16:31 +0000

The name of the modules should be all lowercase, so test_app_launch.py

281 + def test_container_does_not_load_with_no_webapp_name_and_url(self):
312 + def test_container_does_not_load_with_no_webapp_and_url(self):

180 + def launch_webcontainer_app(self):

I think it would be clearer if you don't catch the exception here, and just let it go it's way. Then, you can remove from the tests the
326 + self.assertThat(self.get_webcontainer_proxy(), NotEquals(None))
It will just fail if for some reason the app can't be launched.

review: Needs Fixing (code review)

Revision history for this message

Leo Arias (elopio) wrote on 2014-01-31:

#

Take a look at how we are starting fake servers as fixtures:
http://bazaar.launchpad.net/~ubuntuone-hackers/unity-scope-click/trunk/view/head:/autopilot/unityclickscope/fixture_setup.py

That's nice because it makes the code a little cleaner, but more important, you don't have to manually add the server stop on the tests, so you won't forget it. And, you will be able to test that your assumptions and preconditions are sane.

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-03-13

447. By Launchpad Translations on behalf of phablet-team on 2014-02-06: Launchpad automatic translations update.
448. By Launchpad Translations on behalf of phablet-team on 2014-02-10: Launchpad automatic translations update.
449. By Launchpad Translations on behalf of phablet-team on 2014-02-11: Launchpad automatic translations update.
450. By Launchpad Translations on behalf of phablet-team on 2014-02-12: Launchpad automatic translations update.
451. By Olivier Tilloy on 2014-02-19: Do not install the selection event handlers on certain domains known to interfere with touch events. Fixes: 1279903
452. By PS Jenkins bot on 2014-02-19: Releasing 0.23+14.04.20140219-0ubuntu1
453. By Launchpad Translations on behalf of phablet-team on 2014-02-27: Launchpad automatic translations update.
454. By Alexandre Abreu on 2014-03-04: Add online account support in the webapp container
455. By PS Jenkins bot on 2014-03-04: Releasing 0.23+14.04.20140304-0ubuntu1
456. By Launchpad Translations on behalf of phablet-team on 2014-03-07: Launchpad automatic translations update.
457. By Launchpad Translations on behalf of phablet-team on 2014-03-11: Launchpad automatic translations update.
458. By Launchpad Translations on behalf of phablet-team on 2014-03-12: Launchpad automatic translations update.
459. By Timo Jyrinki on 2014-03-13: Apply the revert that is in archives now, and rebuild against Qt 5.2.1
460. By PS Jenkins bot on 2014-03-13: Releasing 0.23+14.04.20140313-0ubuntu1

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-17:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-03-17:

#

done & fixed

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-17:

#

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

review: Approve (continuous-integration)

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-04-02

461. By Olivier Tilloy on 2014-03-17

Do not register private types in the public plugin.
Register them in the webbrowser application scope instead.

I have made sure that private types that were previously public are currently not being used by any application in the store, so this refactoring is safe.

462. By Leo Arias on 2014-03-17

Updated the copyright header on all the python files.

463. By Adnane Belmadiaf on 2014-03-17

Fix to open URL that start with / (local files)

464. By Olivier Tilloy on 2014-03-17

Refactor the test HTTP server to use a random port at startup.

465. By PS Jenkins bot on 2014-03-17

Releasing 0.23+14.04.20140317-0ubuntu1

466. By Colin Watson on 2014-03-17

Add multiarch annotations.

467. By PS Jenkins bot on 2014-03-17

Releasing 0.23+14.04.20140317.1-0ubuntu1

468. By Olivier Tilloy on 2014-03-19

Use the renderer only once when rendering a thumbnail.
This seems to fix the crash that was happening at destruction time. Fixes: 1294219

469. By PS Jenkins bot on 2014-03-19

Releasing 0.23+14.04.20140319-0ubuntu1

470. By Launchpad Translations on behalf of phablet-team on 2014-03-21

Launchpad automatic translations update.

471. By Alexandre Abreu on 2014-03-24

There are 2 ways for the webapp-container to receive a list of patterns to filter url browsing in a webapp container: through the manifest.json file that comes with a given (named) webapp, or through the command line.

At the moment, the 2 mechanisms are exclusive in the sense that from the webapp-container's perspective it only considers one or another when filtering navigation (the webapp manifest one being preferred) but they are NOT semantically equivalent. The list of url patterns in the manifest.json file is use as a mean to filter the urls that are being browsed to, but ALSO in the chromium & FF browsers to know if a webapp is to be installed/enabled when browsing in a given url.

For example, for Gmail, the hostname mail.google.com is used as a trigger to ask the user for an installation/enablement of a webapp in the browser. It is also implicitely used as a filter for browser once in the container.

The issue is that for applications like google services, the authentication mechanism is common. So when launched in the container, the user will be redirected to e.g. https://accounts.google.com or https://accounts.google.ca, etc.

So, the manifest.json cannot be modified to account for those, since we dont want to install a webapp from those URLs but we need a way to add extra navigation url patterns to allow google service webapps to be redirected to e.g. https://accounts.google.* etc.

We provide here 2 things:

- an way for the url patterns passed down from the command line to be also considered in the navigation filtering,
- a way to simplify url patterns for google apps Fixes: 1288982

472. By PS Jenkins bot on 2014-03-24

Releasing 0.23+14.04.20140324-0ubuntu1

473. By Olivier Tilloy on 2014-03-28

Land the master Oxide support branch. See individual revisions for details. Fixes: 1271756, 1271759

474. By PS Jenkins bot on 2014-03-28

Releasing 0.23+14.04.20140328.3-0ubuntu1

475. By PS Jenkins bot on 2014-04-01

Resync trunk

476. By Launchpad Translations on behalf of phablet-team on 2014-04-02

Launchpad automatic translations update.

477. By Olivier Tilloy on 2014-04-02

Null-check to fix a segmentation fault when --help is passed to the executable.

478. By Alexandre Abreu on 2014-04-02

Small update to the --webapp flag documentation

479. By PS Jenkins bot on 2014-04-02

Releasing 0.23+14.04.20140402-0ubuntu1

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-02:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-02:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-03:

#

FAILED: Continuous integration, rev:477
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/689/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4506/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-amd64-ci/191/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-armhf-ci/191/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-trusty-i386-ci/191/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4615/console

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

review: Needs Fixing (continuous-integration)

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-04-03

480. By Alexandre Abreu on 2014-04-03: merge trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-03:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-04:

#

The copyright headers of the new python files need to be updated to match those of the existing files. See e.g. tests/autopilot/webbrowser_app/__init__.py

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-04:

#

I’m getting 13 errors and warnings when running flake8 on tests/autopilot/webapp_container.
Please fix them.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-04:

#

54 + else {

For consistency with the rest of the code, please keep the else on the same line as the preceding closing brace.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-04:

#

In RequestHandler.serve_content, you call self.send_response(200), but it has already been called in do_GET.

review: Needs Fixing

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-04:

#

For WebappContainerContentHttpServer, you might want to mimick what’s done in HTTPServerInAThread (the implementation was recently changed to not inherit from threading.Thread, which results in better encapsulation and an easier to use class).

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-04:

#

It looks to me like test_container_does_not_load_with_no_webapp_name_and_url and test_container_does_not_load_with_no_webapp_and_url do exactly the same thing. Can of one the two be removed?

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-04-07

481. By Alexandre Abreu on 2014-04-07: update flake8
482. By Alexandre Abreu on 2014-04-07: better comment
483. By Alexandre Abreu on 2014-04-07: fixes

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-04-07:

#

all done

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-04-07

484. By Alexandre Abreu on 2014-04-07: fix typo

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-07:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-07:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-09:

#

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

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-09:

#

199 + self.http_server.shutdown()

instead of the above, for increased robustness (because if an exception happens, tearDown won’t be called), you should do the following in setUp, just after instantiating self.http_server:

self.addCleanup(self.http_server.cleanup)

in which case the tearDown method becomes useless and can be removed.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-09:

#

194 + port = self.http_server.server.server_port

to avoid surfacing an implementation detail of WebappContainerContentHttpServer, you could expose a 'port' property on it, like so:

    class WebappContainerContentHttpServer(object):
        […]
        @property
        def port(self):
            return self.server.server_port

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-04-09:

#

all done

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-04-09

485. By Alexandre Abreu on 2014-04-09: fixes

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-09:

#

265 + if self.server and self.server:

if True and True ?

self.server will never evaluate to False, as it is assigned in __init__. If the instantiation fails, then __init__ will most likely throw an exception, and the test will blow up anyway, so the 'port' property will never be queried. So I think it should be safe to remove this check.

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-04-09

486. By Alexandre Abreu on 2014-04-09: remove cruft

Revision history for this message

Alexandre Abreu (abreu-alexandre) wrote on 2014-04-09:

#

ok done,

lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer updated on 2014-04-09

487. By Alexandre Abreu on 2014-04-09: remove cruft

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-04-09:

#

Looks good now. Thanks!

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-09:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-09:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-11:

#

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

review: Needs Fixing (continuous-integration)

webbrowser-app

Merge lp:~abreu-alexandre/webbrowser-app/add-integration-tests-for-webappcontainer into lp:webbrowser-app

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'debian/control'
 --- debian/control	2014-03-27 16:11:54 +0000
 +++ debian/control	2014-04-09 16:09:26 +0000
@@ -98,3 +98,15 @@
   A lightweight web browser tailored for Ubuntu, based on the Webkit rendering
   engine and using the Ubuntu UI components. This package contains autopilot
   tests for it.
++
++Package: webapp-container-autopilot
++Architecture: all
++Depends: ${misc:Depends},
++         libautopilot-qt (>= 1.4),
++         libqt5test5,
++         ubuntu-ui-toolkit-autopilot,
++         webapp-container (>= ${binary:Version}),
++Description: Ubuntu web applications container autopilot tests
++ A lightweight webapps container tailored for Ubuntu, based on the Webkit
++ rendering engine and using the Ubuntu UI components. This package contains autopilot
++ tests for it.
 === added file 'debian/webapp-container-autopilot.install'
 --- debian/webapp-container-autopilot.install	1970-01-01 00:00:00 +0000
 +++ debian/webapp-container-autopilot.install	2014-04-09 16:09:26 +0000
@@ -0,0 +1,1 @@
++usr/lib/python*/dist-packages/webapp_container/*
 === modified file 'src/app/webcontainer/WebApp.qml'
 --- src/app/webcontainer/WebApp.qml	2014-03-28 16:05:11 +0000
 +++ src/app/webcontainer/WebApp.qml	2014-04-09 16:09:26 +0000
@@ -26,6 +26,7 @@
  BrowserView {
      id: webapp
++    objectName: "webappBrowserView"
      currentWebview: webview.currentWebview
@@ -86,6 +87,7 @@
      PanelLoader {
          id: panel
++	objectName: "panel"
          currentWebview: webview.currentWebview
          chromeless: webapp.chromeless
 === modified file 'src/app/webcontainer/webapp-container.cpp'
 --- src/app/webcontainer/webapp-container.cpp	2014-04-02 15:15:36 +0000
 +++ src/app/webcontainer/webapp-container.cpp	2014-04-09 16:09:26 +0000
@@ -84,7 +84,9 @@
              QList<QUrl> urls = this->urls();
              if (!urls.isEmpty()) {
                  m_window->setProperty("url", urls.first());
--            }
++            } else {
++	        return false;
++	    }
+         }
          m_component->completeCreate();
 === modified file 'src/app/webcontainer/webapp-container.qml'
 --- src/app/webcontainer/webapp-container.qml	2014-03-21 12:47:26 +0000
 +++ src/app/webcontainer/webapp-container.qml	2014-04-09 16:09:26 +0000
@@ -21,6 +21,8 @@
  import Ubuntu.Components 0.1
  Window {
++    objectName: "webappContainer"
++
      property alias developerExtrasEnabled: browser.developerExtrasEnabled
      property alias backForwardButtonsVisible: browser.backForwardButtonsVisible
 === modified file 'tests/autopilot/CMakeLists.txt'
 --- tests/autopilot/CMakeLists.txt	2013-03-08 09:52:30 +0000
 +++ tests/autopilot/CMakeLists.txt	2014-04-09 16:09:26 +0000
@@ -1,9 +1,7 @@
--set(AUTOPILOT_DIR webbrowser_app)
--
  execute_process(COMMAND python -c "from distutils.sysconfig import get_python_lib; print get_python_lib()"
      OUTPUT_VARIABLE PYTHON_PACKAGE_DIR OUTPUT_STRIP_TRAILING_WHITESPACE)
--install(DIRECTORY ${AUTOPILOT_DIR}
--    DESTINATION ${PYTHON_PACKAGE_DIR}
--    )
++install(DIRECTORY webbrowser_app webapp_container
++  DESTINATION ${PYTHON_PACKAGE_DIR}
++  )
 === added directory 'tests/autopilot/webapp_container'
 === added file 'tests/autopilot/webapp_container/__init__.py'
 --- tests/autopilot/webapp_container/__init__.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/webapp_container/__init__.py	2014-04-09 16:09:26 +0000
@@ -0,0 +1,16 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++# Copyright 2014 Canonical
++#
++# 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/>.
++
++"""webapp_container autopilot tests and emulators - top level package."""
 === added directory 'tests/autopilot/webapp_container/tests'
 === added file 'tests/autopilot/webapp_container/tests/__init__.py'
 --- tests/autopilot/webapp_container/tests/__init__.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/webapp_container/tests/__init__.py	2014-04-09 16:09:26 +0000
@@ -0,0 +1,82 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++# Copyright 2014 Canonical
++#
++# 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/>.
++
++""" Autopilot tests for the webapp_container package """
++
++import os
++import subprocess
++
++from autopilot.testcase import AutopilotTestCase
++from autopilot.platform import model
++
++from ubuntuuitoolkit import emulators as toolkit_emulators
++from webapp_container.tests import fake_servers
++
++BASE_FILE_PATH = os.path.dirname(os.path.realpath(__file__))
++CONTAINER_EXEC_REL_PATH = '../../../../src/app/webcontainer/webapp-container'
++INSTALLED_BROWSER_CONTAINER_PATH_NAME = 'webapp-container'
++try:
++    INSTALLED_BROWSER_CONTAINER_PATH_NAME = subprocess.check_output(
++        ['which', 'webapp-container']).strip()
++except subprocess.CalledProcessError:
++    pass
++
++LOCAL_BROWSER_CONTAINER_PATH_NAME = "%s/%s" % (BASE_FILE_PATH,
++                                               CONTAINER_EXEC_REL_PATH)
++
++
++class WebappContainerTestCaseBase(AutopilotTestCase):
++    def get_webcontainer_app_path(self):
++        if os.path.exists(LOCAL_BROWSER_CONTAINER_PATH_NAME):
++            return LOCAL_BROWSER_CONTAINER_PATH_NAME
++        return INSTALLED_BROWSER_CONTAINER_PATH_NAME
++
++    def launch_webcontainer_app(self, args):
++        if model() != 'Desktop':
++            args.append(
++                '--desktop_file_hint=/usr/share/applications/'
++                'webbrowser-app.desktop')
++        try:
++            self.app = self.launch_test_application(
++                self.get_webcontainer_app_path(),
++                *args,
++                emulator_base=toolkit_emulators.UbuntuUIToolkitEmulatorBase)
++        except:
++            self.app = None
++
++    def get_webcontainer_proxy(self):
++        return self.app
++
++    def get_webcontainer_window(self):
++        return self.app.select_single(objectName="webappContainer")
++
++    def get_webcontainer_webview(self):
++        return self.app.select_single(objectName="webappBrowserView")
++
++    def get_webcontainer_panel(self):
++        return self.app.select_single(objectName="panel")
++
++
++class WebappContainerTestCaseWithLocalContentBase(WebappContainerTestCaseBase):
++    def setUp(self):
++        super(WebappContainerTestCaseWithLocalContentBase, self).setUp()
++        self.http_server = fake_servers.WebappContainerContentHttpServer()
++        self.addCleanup(self.http_server.shutdown)
++        self.base_url = "http://localhost:%d" % self.http_server.port
++
++    def launch_webcontainer_app_with_local_http_server(self, args, path='/'):
++        self.url = self.base_url + path
++        args.append(self.url)
++        self.launch_webcontainer_app(args)
 === added file 'tests/autopilot/webapp_container/tests/fake_servers.py'
 --- tests/autopilot/webapp_container/tests/fake_servers.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/webapp_container/tests/fake_servers.py	2014-04-09 16:09:26 +0000
@@ -0,0 +1,68 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++# Copyright 2014 Canonical
++#
++# 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/>.
++
++""" Autopilot tests for the webapp_container package """
++
++import BaseHTTPServer
++import logging
++import threading
++
++
++class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
++    def serve_content(self, content, mime_type='text/html'):
++        self.send_header('Content-type', mime_type)
++        self.end_headers()
++        self.wfile.write(content)
++
++    def basic_html_content(self):
++        return """
++<html>
++<head>
++<title>Some content</title>
++</head>
++<body>
++This is some content
++</body>
++</html>
++        """
++
++    def do_GET(self):
++        if self.path == '/':
++            self.send_response(200)
++            self.serve_content(self.basic_html_content())
++        else:
++            self.send_error(404)
++
++
++class WebappContainerContentHttpServer(object):
++    def __init__(self):
++        super(WebappContainerContentHttpServer, self).__init__()
++        self.server = BaseHTTPServer.HTTPServer(("", 0), RequestHandler)
++        self.server.allow_reuse_address = True
++        self.server_thread = threading.Thread(target=self.server.serve_forever)
++        self.server_thread.start()
++        logging.info("now serving on port {}".format(self.server.server_port))
++
++    @property
++    def port(self):
++        return self.server.server_port
++
++    def run(self):
++        self.server.serve_forever()
++
++    def shutdown(self):
++        self.server.shutdown()
++        self.server.server_close()
++        self.server_thread.join()
 === added file 'tests/autopilot/webapp_container/tests/test_app_launch.py'
 --- tests/autopilot/webapp_container/tests/test_app_launch.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/webapp_container/tests/test_app_launch.py	2014-04-09 16:09:26 +0000
@@ -0,0 +1,36 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++# Copyright 2014 Canonical
++#
++# 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/>.
++
++from __future__ import absolute_import
++
++from testtools.matchers import Equals
++from autopilot.matchers import Eventually
++
++from webapp_container.tests import WebappContainerTestCaseWithLocalContentBase
++
++
++class WebappContainerAppLaunchTestCase(
++        WebappContainerTestCaseWithLocalContentBase):
++
++    def test_container_does_not_load_with_no_webapp_name_and_url(self):
++        args = ['--webapp']
++        self.launch_webcontainer_app(args)
++        self.assertThat(self.get_webcontainer_proxy(), Equals(None))
++
++    def test_loads_with_url(self):
++        args = ['--enable-addressbar']
++        self.launch_webcontainer_app_with_local_http_server(args)
++        self.assertThat(lambda: self.get_webcontainer_window().url,
++                        Eventually(Equals(self.url)))
 === added file 'tests/autopilot/webapp_container/tests/test_chrome_setup.py'
 --- tests/autopilot/webapp_container/tests/test_chrome_setup.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/webapp_container/tests/test_chrome_setup.py	2014-04-09 16:09:26 +0000
@@ -0,0 +1,48 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++# Copyright 2014 Canonical
++#
++# 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/>.
++
++from __future__ import absolute_import
++
++from testtools.matchers import Equals, NotEquals
++
++from webapp_container.tests import WebappContainerTestCaseWithLocalContentBase
++
++
++class WebappContainerChromeSetupTestCase(
++        WebappContainerTestCaseWithLocalContentBase):
++
++    def test_default_to_chromeless(self):
++        self.launch_webcontainer_app_with_local_http_server([])
++        self.assertThat(self.get_webcontainer_proxy(), NotEquals(None))
++        self.assertThat(self.get_webcontainer_webview().chromeless,
++                        Equals(True))
++
++    def test_enable_chrome_back_forward(self):
++        args = ['--enable-back-forward']
++        self.launch_webcontainer_app_with_local_http_server(args)
++        self.assertThat(self.get_webcontainer_webview().chromeless,
++                        Equals(False))
++        panel = self.get_webcontainer_panel()
++        self.assertThat(panel.backForwardButtonsVisible,
++                        Equals(True))
++
++    def test_enable_chrome_address_bar(self):
++        args = ['--enable-addressbar']
++        self.launch_webcontainer_app_with_local_http_server(args)
++        self.assertThat(self.get_webcontainer_proxy(), NotEquals(None))
++        self.assertThat(self.get_webcontainer_webview().chromeless,
++                        Equals(False))
++        self.assertThat(self.get_webcontainer_panel().addressBarVisible,
++                        Equals(True))