You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/11/19 16:36:42 UTC
[GitHub] [airflow] jedcunningham commented on a change in pull request #19712: Rework webserver cli tests to not retest Gunicorn
jedcunningham commented on a change in pull request #19712:
URL: https://github.com/apache/airflow/pull/19712#discussion_r753350432
##########
File path: tests/cli/commands/test_webserver_command.py
##########
@@ -386,63 +346,69 @@ def test_cli_webserver_shutdown_when_gunicorn_master_is_killed(self, _):
webserver_command.webserver(args)
assert ctx.value.code == 1
- def test_cli_webserver_debug(self):
- env = os.environ.copy()
- proc = psutil.Popen(["airflow", "webserver", "--debug"], env=env)
- time.sleep(3) # wait for webserver to start
- return_code = proc.poll()
- assert return_code is None, f"webserver terminated with return code {return_code} in debug mode"
- proc.terminate()
- assert -15 == proc.wait(60)
-
- def test_cli_webserver_access_log_format(self):
-
- # json access log format
- access_logformat = (
- "{\"ts\":\"%(t)s\",\"remote_ip\":\"%(h)s\",\"request_id\":\"%({"
- "X-Request-Id}i)s\",\"code\":\"%(s)s\",\"request_method\":\"%(m)s\","
- "\"request_path\":\"%(U)s\",\"agent\":\"%(a)s\",\"response_time\":\"%(D)s\","
- "\"response_length\":\"%(B)s\"} "
- )
- with tempfile.TemporaryDirectory() as tmpdir, mock.patch.dict(
- "os.environ",
- AIRFLOW__CORE__DAGS_FOLDER="/dev/null",
- AIRFLOW__CORE__LOAD_EXAMPLES="False",
- AIRFLOW__WEBSERVER__WORKERS="1",
- ):
- access_logfile = f"{tmpdir}/access.log"
- # Run webserver in foreground and terminate it.
+ def test_cli_webserver_debug(self, app):
+ with mock.patch.object(webserver_command, 'create_app') as create_app, mock.patch.object(
+ app, 'run'
+ ) as app_run:
+ create_app.return_value = app
- proc = subprocess.Popen(
+ args = self.parser.parse_args(
+ [
+ "webserver",
+ "--debug",
+ ]
+ )
+ webserver_command.webserver(args)
+
+ app_run.assert_called_with(
+ debug=True,
+ use_reloader=False,
+ port=8080,
+ host='0.0.0.0',
+ ssl_context=None,
+ )
+
+ def test_cli_webserver_args(self):
+ access_logformat = "custom_log_format"
+
+ with mock.patch("subprocess.Popen") as Popen, mock.patch.object(webserver_command, 'GunicornMonitor'):
+ args = self.parser.parse_args(
[
- "airflow",
"webserver",
- "--access-logfile",
- access_logfile,
"--access-logformat",
access_logformat,
+ "--pid",
+ "/tmp/x.pid",
]
)
- assert proc.poll() is None
+ webserver_command.webserver(args)
- # Wait for webserver process
- time.sleep(10)
-
- proc2 = subprocess.Popen(["curl", "http://localhost:8080"])
- proc2.wait(10)
- try:
- with open(access_logfile) as file:
- log = json.loads(file.read())
- assert '127.0.0.1' == log.get('remote_ip')
- assert len(log) == 9
- assert 'GET' == log.get('request_method')
-
- except OSError:
- print("access log file not found at " + access_logfile)
-
- # Terminate webserver
- proc.terminate()
- # -15 - the server was stopped before it started
- # 0 - the server terminated correctly
- assert proc.wait(60) in (-15, 0)
- self._check_processes()
+ Popen.assert_called_with(
+ [
+ sys.executable,
+ '-m',
+ 'gunicorn',
+ '--workers',
+ '4',
+ '--worker-class',
+ 'sync',
+ '--timeout',
+ '120',
+ '--bind',
+ '0.0.0.0:8080',
+ '--name',
+ 'airflow-webserver',
+ '--pid',
+ '/tmp/x.pid',
+ '--config',
+ 'python:airflow.www.gunicorn_config',
+ '--access-logfile',
+ '-',
+ '--error-logfile',
+ '-',
+ '--access-logformat',
+ access_logformat,
Review comment:
```suggestion
'custom_log_format',
```
##########
File path: tests/cli/commands/test_webserver_command.py
##########
@@ -386,63 +346,69 @@ def test_cli_webserver_shutdown_when_gunicorn_master_is_killed(self, _):
webserver_command.webserver(args)
assert ctx.value.code == 1
- def test_cli_webserver_debug(self):
- env = os.environ.copy()
- proc = psutil.Popen(["airflow", "webserver", "--debug"], env=env)
- time.sleep(3) # wait for webserver to start
- return_code = proc.poll()
- assert return_code is None, f"webserver terminated with return code {return_code} in debug mode"
- proc.terminate()
- assert -15 == proc.wait(60)
-
- def test_cli_webserver_access_log_format(self):
-
- # json access log format
- access_logformat = (
- "{\"ts\":\"%(t)s\",\"remote_ip\":\"%(h)s\",\"request_id\":\"%({"
- "X-Request-Id}i)s\",\"code\":\"%(s)s\",\"request_method\":\"%(m)s\","
- "\"request_path\":\"%(U)s\",\"agent\":\"%(a)s\",\"response_time\":\"%(D)s\","
- "\"response_length\":\"%(B)s\"} "
- )
- with tempfile.TemporaryDirectory() as tmpdir, mock.patch.dict(
- "os.environ",
- AIRFLOW__CORE__DAGS_FOLDER="/dev/null",
- AIRFLOW__CORE__LOAD_EXAMPLES="False",
- AIRFLOW__WEBSERVER__WORKERS="1",
- ):
- access_logfile = f"{tmpdir}/access.log"
- # Run webserver in foreground and terminate it.
+ def test_cli_webserver_debug(self, app):
+ with mock.patch.object(webserver_command, 'create_app') as create_app, mock.patch.object(
+ app, 'run'
+ ) as app_run:
+ create_app.return_value = app
- proc = subprocess.Popen(
+ args = self.parser.parse_args(
+ [
+ "webserver",
+ "--debug",
+ ]
+ )
+ webserver_command.webserver(args)
+
+ app_run.assert_called_with(
+ debug=True,
+ use_reloader=False,
+ port=8080,
+ host='0.0.0.0',
+ ssl_context=None,
+ )
+
+ def test_cli_webserver_args(self):
+ access_logformat = "custom_log_format"
Review comment:
```suggestion
```
nit: probably not worth a separate variable?
##########
File path: tests/cli/commands/test_webserver_command.py
##########
@@ -386,63 +346,69 @@ def test_cli_webserver_shutdown_when_gunicorn_master_is_killed(self, _):
webserver_command.webserver(args)
assert ctx.value.code == 1
- def test_cli_webserver_debug(self):
- env = os.environ.copy()
- proc = psutil.Popen(["airflow", "webserver", "--debug"], env=env)
- time.sleep(3) # wait for webserver to start
- return_code = proc.poll()
- assert return_code is None, f"webserver terminated with return code {return_code} in debug mode"
- proc.terminate()
- assert -15 == proc.wait(60)
-
- def test_cli_webserver_access_log_format(self):
-
- # json access log format
- access_logformat = (
- "{\"ts\":\"%(t)s\",\"remote_ip\":\"%(h)s\",\"request_id\":\"%({"
- "X-Request-Id}i)s\",\"code\":\"%(s)s\",\"request_method\":\"%(m)s\","
- "\"request_path\":\"%(U)s\",\"agent\":\"%(a)s\",\"response_time\":\"%(D)s\","
- "\"response_length\":\"%(B)s\"} "
- )
- with tempfile.TemporaryDirectory() as tmpdir, mock.patch.dict(
- "os.environ",
- AIRFLOW__CORE__DAGS_FOLDER="/dev/null",
- AIRFLOW__CORE__LOAD_EXAMPLES="False",
- AIRFLOW__WEBSERVER__WORKERS="1",
- ):
- access_logfile = f"{tmpdir}/access.log"
- # Run webserver in foreground and terminate it.
+ def test_cli_webserver_debug(self, app):
+ with mock.patch.object(webserver_command, 'create_app') as create_app, mock.patch.object(
+ app, 'run'
+ ) as app_run:
+ create_app.return_value = app
- proc = subprocess.Popen(
+ args = self.parser.parse_args(
+ [
+ "webserver",
+ "--debug",
+ ]
+ )
+ webserver_command.webserver(args)
+
+ app_run.assert_called_with(
+ debug=True,
+ use_reloader=False,
+ port=8080,
+ host='0.0.0.0',
+ ssl_context=None,
+ )
+
+ def test_cli_webserver_args(self):
+ access_logformat = "custom_log_format"
+
+ with mock.patch("subprocess.Popen") as Popen, mock.patch.object(webserver_command, 'GunicornMonitor'):
+ args = self.parser.parse_args(
[
- "airflow",
"webserver",
- "--access-logfile",
- access_logfile,
"--access-logformat",
access_logformat,
Review comment:
```suggestion
"custom_log_format",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org