issue 3546 - Add port and hostname options to CMS testing server

cms/bin/test_server.py

Index: cms/bin/test_server.py
===================================================================
--- a/cms/bin/test_server.py
+++ b/cms/bin/test_server.py
@@ -18,6 +18,7 @@
 import mimetypes
 import os
 import sys
+import argparse
 
 import jinja2
 
@@ -26,6 +27,8 @@
 from cms.converters import converters
 
 source = None
+address = 'localhost'

[Sebastian Noack, 2016/01/21 14:14:09]

Please don't define defaults redundantly. You can simply set address and port to
None here. It will get overridden when the arguments have been parsed anyway.

[juliandoucette, 2016/01/21 14:41:35]

Acknowledged.

+port = '5000'
 
 UNICODE_ENCODING = "utf-8"
 
@@ -65,7 +68,7 @@
   for format in converters.iterkeys():
     for p in (page, alternative_page):
       if source.has_page(p, format):
-        return (p, process_page(source, locale, p, format, "http://127.0.0.1:5000"))
+        return (p, process_page(source, locale, p, format, "http://" + address + ":" + str(port)))

[Sebastian Noack, 2016/01/21 14:14:08]

Nit: Please use format strings when concatenating more than one string:

  "http://%s:%d" % (address, port)

[juliandoucette, 2016/01/21 14:41:35]

Acknowledged.

   if source.has_localizable_file(locale, page):
     return (page, source.read_localizable_file(locale, page))
 
@@ -99,6 +102,7 @@
     yield fragment.encode(UNICODE_ENCODING)
 
 def handler(environ, start_response):
+  
   path = environ.get("PATH_INFO")
 
   data = get_data(path)
@@ -115,12 +119,23 @@
   return [data]
 
 if __name__ == "__main__":
-  if len(sys.argv) < 2:
-    source = FileSource(os.curdir)
-  elif os.path.isdir(sys.argv[1]):
-    source = FileSource(sys.argv[1])
-  else:
-    sys.exit("Usage: %s [source_dir]" % sys.argv[0])
+  
+  parser = argparse.ArgumentParser(description='CMS testing server.')
+  parser.add_argument('path', nargs='?', default=os.curdir)
+  parser.add_argument('-n', '--address', default='localhost', help='Address of the interface the server will listen to.')

[Sebastian Noack, 2016/01/21 14:14:08]

Nit: It's semantically not a full sentence. So no full stop.
Nit: I think it should be "listen on".

[juliandoucette, 2016/01/21 14:41:36]

Acknowledged.

+  parser.add_argument('-p', '--port', type=int, default=5000, help='Port number.')

[Sebastian Noack, 2016/01/21 14:14:09]

Nit: This help text doesn't tell anything that the option wouldn't already tell
by it's name. Better: "The TCP port the server will listen on".

[juliandoucette, 2016/01/21 14:41:36]

Acknowledged.

+  parser.add_argument('-l', '--listen', help='Address and port number separated by a semicolin.')

[Sebastian Noack, 2016/01/21 14:14:08]

I didn't meant to implement both, --address/--port and --listen, but only either
of them. It's usually not a good thing to provide two ways to achieve the same
thing. And the added complexity to support both isn't certainly worth it. I
leave it up to you which one you will go with.

[juliandoucette, 2016/01/21 14:41:35]

- Sorry, I misunderstood
- I agree
- I will go with --address/--port because it is more straightforward 

+  args = parser.parse_args()
+  
+  source = FileSource(args.path)
+  address = args.address
+  port = args.port
+  
+  if (args.listen):
+    listen = str.split(args.listen, ':')

[Sebastian Noack, 2016/01/21 14:14:08]

Nit: args.listen.split(":")

[juliandoucette, 2016/01/21 14:41:35]

Acknowledged.

+    if (len(listen) > 1):
+      port = int(listen[1])
+    address = listen[0]
 
   try:
     from werkzeug.serving import ThreadedWSGIServer, run_simple
@@ -158,4 +173,4 @@
       print " * Running on http://%s:%i/" % server.server_address
       server.serve_forever()
 
-  run("localhost", 5000, handler, use_reloader=True, use_debugger=True)
+  run(address, port, handler, use_reloader=True, use_debugger=True)