Repository: beancount/fava
Issue: Change default port (at least on macOS)

macOS now uses port 5000 by default for AirPlay discovery (https://news.ycombinator.com/item?id=29006861).
Attempting to start Fava with the default port now fails silently on macOS (it says it's running, but the web interface doesn't load because the port is already in use by the system).

Port 5000 is in any case already a reserved port for UPnP:
- https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?&page=88
- https://www.grc.com/port_5000.htm

It would be better to use a port in the 8000+ range (and also to detect if the port is already in use and fall back to a higher port).

Changed files:
tests/conftest.py, tests/test_cli.py, src/fava/cli.py

diff --git a/src/fava/cli.py b/src/fava/cli.py
index c940b74b..44de63df 100644
--- a/src/fava/cli.py
+++ b/src/fava/cli.py
@@ -94,10 +94,25 @@ def main(
             simple_wsgi, {prefix: app.wsgi_app}
         )
 
+    if host == "localhost":
+        # ensure that cheroot does not use IP6 for localhost
+        host = "127.0.0.1"
+
+    click.echo(f"Starting Fava on http://{host}:{port}")
     if not debug:
         server = Server((host, port), app)
-        print(f"Running Fava on http://{host}:{port}")
-        server.safe_start()
+        try:
+            server.start()
+        except KeyboardInterrupt:
+            click.echo("Keyboard interrupt received: stopping Fava", err=True)
+            server.stop()
+        except OSError as error:
+            if "No socket could be created" in str(error):
+                click.echo(
+                    f"Cannot start Fava because port {port} is already in use."
+                    "\\nPlease choose a different port with the '-p' option."
+                )
+            raise click.Abort
     else:
         if profile:
             app.config["PROFILE"] = True
@@ -112,10 +127,11 @@ def main(
             app.run(host, port, debug)
         except OSError as error:
             if error.errno == errno.EADDRINUSE:
-                raise click.UsageError(
-                    "Can not start webserver because the port is already in "
-                    "use. Please choose another port with the '-p' option."
+                click.echo(
+                    f"Cannot start Fava because port {port} is already in use."
+                    "\\nPlease choose a different port with the '-p' option."
                 )
+                raise click.Abort
             raise
 
 
diff --git a/tests/conftest.py b/tests/conftest.py
index 5c41ed35..0b7f400d 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -32,16 +32,17 @@ def data_file(filename: str) -> str:
     return str(TESTS_DIR / "data" / filename)
 
 
-EXAMPLE_FILE = data_file("long-example.beancount")
+LONG_EXAMPLE_FILE = data_file("long-example.beancount")
+EXAMPLE_FILE = data_file("example.beancount")
 
-EXAMPLE_LEDGER = FavaLedger(EXAMPLE_FILE)
+EXAMPLE_LEDGER = FavaLedger(LONG_EXAMPLE_FILE)
 
 fava_app.testing = True
 TEST_CLIENT = fava_app.test_client()
 
 fava_app.config["BEANCOUNT_FILES"] = [
+    LONG_EXAMPLE_FILE,
     EXAMPLE_FILE,
-    data_file("example.beancount"),
     data_file("extension-report-example.beancount"),
     data_file("import.beancount"),
     data_file("query-example.beancount"),
@@ -79,12 +80,12 @@ def snapshot(request: FixtureRequest) -> SnapshotFunc:
         # print strings directly, otherwise try pretty-printing
         out = data if isinstance(data, str) else pformat(data)
         out = out.replace(
-            EXAMPLE_FILE,
+            LONG_EXAMPLE_FILE,
             "FAVA_LONG_EXAMPLE_PATH.beancount",
         )
         if os.name == "nt":
             out = out.replace(
-                EXAMPLE_FILE.replace("\\\\", "\\\\\\\\"),
+                LONG_EXAMPLE_FILE.replace("\\\\", "\\\\\\\\"),
                 "FAVA_LONG_EXAMPLE_PATH.beancount",
             )
         if not snap_file.exists():
diff --git a/tests/test_cli.py b/tests/test_cli.py
index c6f7f945..d03e43de 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -1,7 +1,6 @@
 # pylint: disable=missing-docstring
 from __future__ import annotations
 
-import os
 import sys
 from socket import socket
 from subprocess import PIPE
@@ -11,14 +10,11 @@ from time import sleep
 from time import time
 
 import pytest
+from pytest import MonkeyPatch
 
 from .conftest import EXAMPLE_FILE
 
 HOST = "0.0.0.0"
-FAVA = ("fava",)
-
-if "BEANCOUNT_FILE" in os.environ:
-    del os.environ["BEANCOUNT_FILE"]
 
 
 def get_port() -> int:
@@ -41,18 +37,19 @@ def output_contains(process: Popen[str], output: str, timeout: int) -> bool:
 
 
 @pytest.mark.skipif(sys.platform == "win32", reason="does not run on windows")
-def test_cli() -> None:
+def test_cli(monkeypatch: MonkeyPatch) -> None:
     port = str(get_port())
-    args = (EXAMPLE_FILE, "-d", "-p", port)
+    monkeypatch.delenv("BEANCOUNT_FILE", raising=False)
+    args = ("fava", EXAMPLE_FILE, "-p", port)
     with Popen(
-        FAVA + args,
+        args,
         stdout=PIPE,
         stderr=STDOUT,
         universal_newlines=True,
     ) as process:
-        assert output_contains(process, "Running on", 20)
+        assert output_contains(process, "Starting Fava on", 20)
         with Popen(
-            FAVA + args,
+            args,
             stdout=PIPE,
             stderr=STDOUT,
             universal_newlines=True,
@@ -61,4 +58,4 @@ def test_cli() -> None:
             process.terminate()
             assert process2.stdout
             assert "in use" in "".join(process2.stdout.readlines())
-            assert process2.returncode == 2
+            assert process2.returncode > 0