uranusjr commented on code in PR #33914:
URL: https://github.com/apache/airflow/pull/33914#discussion_r1312698340
##########
tests/providers/docker/operators/test_docker.py:
##########
@@ -553,19 +553,19 @@ def test_skip(self, extra_kwargs, actual_exit_code,
expected_exc):
def test_execute_container_fails(self):
failed_msg = {"StatusCode": 1}
log_line = ["unicode container log 😁 ", b"byte string container log"]
- expected_message = "Docker container failed: {failed_msg} lines
{expected_log_output}"
+ expected_message = "Docker container failed: {failed_msg}"
self.client_mock.attach.return_value = log_line
self.client_mock.wait.return_value = failed_msg
operator = DockerOperator(image="ubuntu", owner="unittest",
task_id="unittest")
- with pytest.raises(AirflowException) as raised_exception:
+ with pytest.raises(DockerContainerFailedException) as raised_exception:
operator.execute(None)
- assert str(raised_exception.value) == expected_message.format(
- failed_msg=failed_msg,
-
expected_log_output=f'{log_line[0].strip()}\n{log_line[1].decode("utf-8")}',
- )
+ assert str(raised_exception.value) == expected_message.format(
+ failed_msg=failed_msg,
+ )
+ assert raised_exception.logs ==
f'{log_line[0].strip()}\n{log_line[1].decode("utf-8")}'
Review Comment:
```suggestion
assert raised_exception.value.logs ==
f'{log_line[0].strip()}\n{log_line[1].decode("utf-8")}'
```
Maybe we want to change the name `expected_message` as well… that is not the
actual exception object, but an `ExceptionInfo` instance
https://docs.pytest.org/en/4.6.x/reference.html#pytest-raises
##########
tests/providers/docker/operators/test_docker.py:
##########
@@ -553,19 +553,19 @@ def test_skip(self, extra_kwargs, actual_exit_code,
expected_exc):
def test_execute_container_fails(self):
failed_msg = {"StatusCode": 1}
log_line = ["unicode container log 😁 ", b"byte string container log"]
- expected_message = "Docker container failed: {failed_msg} lines
{expected_log_output}"
+ expected_message = "Docker container failed: {failed_msg}"
self.client_mock.attach.return_value = log_line
self.client_mock.wait.return_value = failed_msg
operator = DockerOperator(image="ubuntu", owner="unittest",
task_id="unittest")
- with pytest.raises(AirflowException) as raised_exception:
+ with pytest.raises(DockerContainerFailedException) as raised_exception:
operator.execute(None)
- assert str(raised_exception.value) == expected_message.format(
- failed_msg=failed_msg,
-
expected_log_output=f'{log_line[0].strip()}\n{log_line[1].decode("utf-8")}',
- )
+ assert str(raised_exception.value) == expected_message.format(
+ failed_msg=failed_msg,
+ )
+ assert raised_exception.logs ==
f'{log_line[0].strip()}\n{log_line[1].decode("utf-8")}'
Review Comment:
```suggestion
assert raised_exception.value.logs ==
f'{log_line[0].strip()}\n{log_line[1].decode("utf-8")}'
```
Maybe we want to change the name `expected_message` as well… that is not the
actual exception object, but an `ExceptionInfo` instance
https://docs.pytest.org/en/4.6.x/reference.html#pytest-raises
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
