Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(342)

Issue 1037733002: Flesh out MojoUrlRedirector apptests (Closed)

Created:
5 years, 9 months ago by blundell
Modified:
5 years, 8 months ago
Reviewers:
qsr, ppi
CC:
mojo-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org
Base URL:
https://github.com/domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Flesh out MojoUrlRedirector apptests This CL adds the following tests of the MojoUrlRedirector: - When asked for an app whose location file is missing, produces 404 response. - When asked for an app on a known platform, returns a redirect to the location contained in the app's location file for that platform. To implement these tests, the MojoUrlRedirector now allows for parameterizing the address that it asks for apps' location files from. The test spins up an HttpServer on localhost, registers an HttpHandler that processes requests for app location files, and instructs the MojoUrlRedirector to talk to that server to obtain app location files. R=ppi@chromium.org Committed: https://chromium.googlesource.com/external/mojo/+/b1c5dd67e3d3b64fccaac879ab33d4ab703a240a

Patch Set 1 #

Patch Set 2 : Cleanup pass #

Patch Set 3 : Update args #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+213 lines, -57 lines) Patch
M mojo/tools/data/apptests View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M services/python/mojo_url_redirector/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M services/python/mojo_url_redirector/__mojo__.py View 1 5 chunks +12 lines, -8 lines 2 comments Download
M services/python/mojo_url_redirector/mojo_url_redirector_apptest.cc View 1 3 chunks +196 lines, -47 lines 1 comment Download

Messages

Total messages: 6 (2 generated)
blundell
5 years, 9 months ago (2015-03-25 11:01:40 UTC) #2
ppi
lgtm As discussed off-bug, it would be interesting to explore asynchronous redirection initialization, e.g. using ...
5 years, 8 months ago (2015-03-30 12:15:16 UTC) #3
blundell
Committed patchset #3 (id:40001) manually as b1c5dd67e3d3b64fccaac879ab33d4ab703a240a (presubmit successful).
5 years, 8 months ago (2015-04-01 10:04:00 UTC) #4
qsr
5 years, 8 months ago (2015-04-01 13:49:03 UTC) #6
Message was sent while issue was closed.
https://codereview.chromium.org/1037733002/diff/40001/services/python/mojo_ur...
File services/python/mojo_url_redirector/__mojo__.py (right):

https://codereview.chromium.org/1037733002/diff/40001/services/python/mojo_ur...
services/python/mojo_url_redirector/__mojo__.py:50: custom_headers["location"] =
new_location
Usually Location (capital L) is used there.

https://codereview.chromium.org/1037733002/diff/40001/services/python/mojo_ur...
services/python/mojo_url_redirector/__mojo__.py:150: assert
len(application.args) >= 2
You might want to use argparse instead of place, this is quickly becoming
unmanageable...

https://codereview.chromium.org/1037733002/diff/40001/services/python/mojo_ur...
File services/python/mojo_url_redirector/mojo_url_redirector_apptest.cc (right):

https://codereview.chromium.org/1037733002/diff/40001/services/python/mojo_ur...
services/python/mojo_url_redirector/mojo_url_redirector_apptest.cc:84: for
(const std::string& arg : application_impl()->args()) {
Given that you are using base, you can probably set the command line using your
args, and then use all the parsing utility that we have to find the parameter
you need.

base::CommandLine::StringVector command_line_args = app->args();
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
command_line->InitFromArgv(command_line_args);

Powered by Google App Engine
This is Rietveld 408576698