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 2020/04/21 22:46:02 UTC
[GitHub] [airflow] mik-laj opened a new pull request #8505: Add more precise tests for AirflowVersion
mik-laj opened a new pull request #8505:
URL: https://github.com/apache/airflow/pull/8505
Recently, I talked to Kaxil about testing our views. I suggested that we should test templates and flask views separately I have prepared a POC that shows how it can be done. I think this can increase confidence in these tests. We are currently testing only random words, but very much logic has no coverage and it is very difficult to make changes. If you want to make changes, you always have to test everything manually to see if the logic is behaving correctly. If we have automatic tests with a higher level of confidence then the changes will be easier to review and we will trust them more.
---
Make sure to mark the boxes below before creating PR: [x]
- [X] Description above provides context of the change
- [X] Unit tests coverage for changes (not needed for documentation changes)
- [X] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [X] Relevant documentation is updated including usage instructions.
- [X] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8505: [POC] Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617505280
I like this, I tested it with:
```python
@mock.patch('airflow.www.views.dagbag.get_dag')
def test_extra_link_in_gantt_view(self, get_dag_function):
from tests.test_utils.mock_operators import Dummy2TestOperator
dag = DAG('dag', start_date=self.default_date)
Dummy2TestOperator(task_id="some_dummy_task_2", dag=dag)
get_dag_function.return_value = dag
exec_date = dates.days_ago(2)
start_date = datetime(2020, 4, 10, 2, 0, 0)
end_date = exec_date + timedelta(seconds=30)
with create_session() as session:
for task in dag.tasks:
ti = TaskInstance(task=task, execution_date=exec_date, state="success")
ti.start_date = start_date
ti.end_date = end_date
session.add(ti)
url = 'gantt?dag_id={}&execution_date={}'.format(self.dag.dag_id, exec_date)
with catch_jinja_params() as jinja_params:
self.client.get(url, follow_redirects=True)
self.assertEqual(jinja_params.template_name, 'airflow/gantt.html')
self.assertCountEqual(
jinja_params.template_params['data']['tasks'][0]['extraLinks'],
['airflow', 'github'])
```
and works fine. However I have 1 question: Do we lost any Airflow related args/customizations because of `side_effect=BaseView.render_template` ??
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on issue #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-618329645
@ashb we commonly use mock and this is a proven solution that we know well. However, I will look at this flask-testing. Thanks for the answer.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on issue #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-618427120
Using the suggested answer from the SO post has some advantages:
1. The context manager code is simpler.
2. The test code is more "direct" too (at least to my eyes):
```python
@mock.patch('airflow.www.views.dagbag.get_dag')
@mock.patch('airflow.www.views.BaseView.render_template')
def test_extra_link_in_gantt_view(self, mock_render_template, get_dag_function):
from tests.test_utils.mock_operators import Dummy2TestOperator
dag = DAG('ex_dag', start_date=self.default_date)
task = Dummy2TestOperator(task_id="some_dummy_task_2", dag=dag)
get_dag_function.return_value = dag
exec_date = dates.days_ago(2)
start_date = datetime(2020, 4, 10, 2, 0, 0)
end_date = exec_date + timedelta(seconds=30)
with create_session() as session:
for task in dag.tasks:
ti = TaskInstance(task=task, execution_date=exec_date, state="success")
ti.start_date = start_date
ti.end_date = end_date
session.add(ti)
with captured_templates(self.app) as jinja_params:
url = 'gantt?dag_id={}&execution_date={}'.format(dag.dag_id, exec_date)
self.client.get(url, follow_redirects=True)
assert len(templates) == 1
template, context = templates[0]
assert template.name == 'index.html'
assert context['data'] == {
'taskNames': ['some_dummy_task_2'],
'tasks': [
{
'task_id': 'some_dummy_task_2',
'dag_id': 'ex_dag',
'execution_date': '2020-04-19T00:00:00+00:00',
'start_date': '2020-04-10T02:00:00+00:00', 'end_date': '2020-04-19T00:00:30+00:00',
'duration': None, 'state': 'success', 'try_number': 1,
'max_tries': 0, 'hostname': '', 'unixname': 'root',
'job_id': None, 'pool': 'default_pool', 'pool_slots': 1, 'queue': 'default',
'priority_weight': 1,
'operator': 'Dummy2TestOperator', 'queued_dttm': None,
'pid': None, 'executor_config': {},
'extraLinks': ['github', 'airflow']
}
], 'height': 50
}
)
```
(untested though)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617786816
Love it as well.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #8505: Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-624593096
Looks great!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8505:
URL: https://github.com/apache/airflow/pull/8505#discussion_r419167999
##########
File path: setup.py
##########
@@ -427,6 +427,7 @@ def write_version(filename: str = os.path.join(*[my_dir, "airflow", "git_version
############################################################################################################
devel = [
'beautifulsoup4~=4.7.1',
+ 'blinker',
Review comment:
Required by flask: https://github.com/pallets/flask/blob/master/src/flask/signals.py#L13
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #8505:
URL: https://github.com/apache/airflow/pull/8505#discussion_r413657261
##########
File path: tests/test_utils/asserts.py
##########
@@ -71,3 +72,33 @@ def assert_queries_count(expected_count, message_fmt=None):
"The current number is {current_count}."
message = message_fmt.format(current_count=result.count, expected_count=expected_count)
assert expected_count == result.count, message
+
+
+class CatchJinjaParmasResult:
+ def __init__(self):
+ self.template_name = None
+ self.template_params = None
+
+
+@contextmanager
+def catch_jinja_params():
Review comment:
And rather than a separate method/in utils, I would suggest making this a method on the already existing TestBase class -- after all it is only useful if you have a flask app, and TestBase sets that up for us.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8505: [POC] Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617470493
> We can create context - `catch_jinja_params()` (similar to `count_queries`, `contextlib.redirect_stdout(io.StringIO())` ) that will help us introduce these assertions into current tests. Thanks to this, we will still have assertions for the HTML code, but also more precise for the Jinja params only.
>
> ```python
> class TestVersionView(TestBase):
> def test_version(self):
> with catch_jinja_params() as jinja_params:
> resp = self.client.get('version', data=dict(
> username='test',
> password='test'
> ), follow_redirects=True)
> self.check_content_in_response('Version Info', resp)
>
> self.assertEqual(jinja_params.template_name, 'airflow/version.html')
> self.assertEqual(jinja_params.params, dict(
> airflow_version=version.version,
> git_version=mock.ANY,
> scheduler_job=mock.ANY,
> title='Version Info'
> ))
> ```
>
> However, I only wanted to present the general concept in this PR.
I like the idea. Without that we would have to do it like the following which works too but less ideal:
```python
@mock.patch('airflow.www.views.dagbag.get_dag')
@mock.patch('airflow.www.views.BaseView.render_template')
def test_extra_link_in_gantt_view(self, mock_render_template, get_dag_function):
from tests.test_utils.mock_operators import Dummy2TestOperator
dag = DAG('ex_dag', start_date=self.default_date)
Dummy2TestOperator(task_id="some_dummy_task_2", dag=dag)
get_dag_function.return_value = dag
exec_date = dates.days_ago(2)
start_date = datetime(2020, 4, 10, 2, 0, 0)
end_date = exec_date + timedelta(seconds=30)
with create_session() as session:
for task in dag.tasks:
ti = TaskInstance(task=task, execution_date=exec_date, state="success")
ti.start_date = start_date
ti.end_date = end_date
session.add(ti)
with self.app.app_context():
mock_render_template.return_value = make_response("RESPONSE", 200)
url = 'gantt?dag_id={}&execution_date={}'.format(dag.dag_id, exec_date)
self.client.get(url, follow_redirects=True, data=dict(
username='test',
password='test'
))
self.assertCountEqual(
mock_render_template.call_args[1]['data']['tasks'][0]['extraLinks'],
['airflow', 'github']
)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil edited a comment on issue #8505: [POC] Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617470493
> We can create context - `catch_jinja_params()` (similar to `count_queries`, `contextlib.redirect_stdout(io.StringIO())` ) that will help us introduce these assertions into current tests. Thanks to this, we will still have assertions for the HTML code, but also more precise for the Jinja params only.
>
> ```python
> class TestVersionView(TestBase):
> def test_version(self):
> with catch_jinja_params() as jinja_params:
> resp = self.client.get('version', data=dict(
> username='test',
> password='test'
> ), follow_redirects=True)
> self.check_content_in_response('Version Info', resp)
>
> self.assertEqual(jinja_params.template_name, 'airflow/version.html')
> self.assertEqual(jinja_params.params, dict(
> airflow_version=version.version,
> git_version=mock.ANY,
> scheduler_job=mock.ANY,
> title='Version Info'
> ))
> ```
>
> However, I only wanted to present the general concept in this PR.
I like the idea. Without that we would have to do it like the following which works too but less ideal:
```python
@mock.patch('airflow.www.views.dagbag.get_dag')
@mock.patch('airflow.www.views.BaseView.render_template')
def test_extra_link_in_gantt_view(self, mock_render_template, get_dag_function):
from tests.test_utils.mock_operators import Dummy2TestOperator
dag = DAG('ex_dag', start_date=self.default_date)
Dummy2TestOperator(task_id="some_dummy_task_2", dag=dag)
get_dag_function.return_value = dag
exec_date = dates.days_ago(2)
start_date = datetime(2020, 4, 10, 2, 0, 0)
end_date = exec_date + timedelta(seconds=30)
with create_session() as session:
for task in dag.tasks:
ti = TaskInstance(task=task, execution_date=exec_date, state="success")
ti.start_date = start_date
ti.end_date = end_date
session.add(ti)
with self.app.app_context():
mock_render_template.return_value = make_response("RESPONSE", 200)
url = 'gantt?dag_id={}&execution_date={}'.format(dag.dag_id, exec_date)
self.client.get(url, follow_redirects=True, data=dict(
username='test',
password='test'
))
self.assertCountEqual(
mock_render_template.call_args[1]['data']['tasks'][0]['extraLinks'],
['airflow', 'github']
)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on issue #8505: [POC] Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617510026
@kaxil We don't lose anything because we replace the method in the BaseView class with mock, and then mock calls the original method. As a result, we have a wrapped(transparent) method. If any class inherits from BaseView, it must use super () for mock to be called. Please note that `scheduler_job` key appears in the tests, which is added in another class, i.e. not VersionView.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on issue #8505: Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617467054
We can create context - `catch_jinja_params()` (similar to `count_queries`, `contextlib.redirect_stdout(io.StringIO())` ) that will help us introduce these assertions into current tests. Thanks to this, we will still have assertions for the HTML code, but also more precise for the Jinja params only.
```python
class TestVersionView(TestBase):
def test_version(self):
with catch_jinja_params() as jinja_params:
resp = self.client.get('version', data=dict(
username='test',
password='test'
), follow_redirects=True)
self.check_content_in_response('Version Info', resp)
self.assertEqual(jinja_params.template_name, 'airflow/version.html')
self.assertEqual(jinja_params.params, dict(
airflow_version=version.version,
git_version=mock.ANY,
scheduler_job=mock.ANY,
title='Version Info'
))
```
However, I only wanted to present the general concept in this PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on issue #8505: Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617467054
We can create context - `catch_jinja_params()` (similar to `count_queries`, `contextlib.redirect_stdout(io.StringIO())` ) that will help us introduce these assertions into current tests.
```python
class TestVersionView(TestBase):
def test_version(self):
with catch_jinja_params() as jinja_params:
resp = self.client.get('version', data=dict(
username='test',
password='test'
), follow_redirects=True)
self.check_content_in_response('Version Info', resp)
self.assertEqual(jinja_params.template_name, 'airflow/version.html')
self.assertEqual(jinja_params.params, dict(
airflow_version=version.version,
git_version=mock.ANY,
scheduler_job=mock.ANY,
title='Version Info'
))
```
However, I only wanted to present the general concept in this PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #8505: Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-624386596
@potiuk Do you have any comments? I rewrote the whole PR from scratch, so you can change your opinions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8505: Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617461651
I recreated the Extra Links PR:
```python
@mock.patch('airflow.www.views.dagbag.get_dag')
@mock.patch('airflow.www.views.BaseView.render_template')
def test_extra_link_in_gantt_view(self, mock_render_template, get_dag_function):
from tests.test_utils.mock_operators import Dummy2TestOperator
dag = DAG('ex_dag', start_date=self.default_date)
task = Dummy2TestOperator(task_id="some_dummy_task_2", dag=dag)
get_dag_function.return_value = dag
exec_date = dates.days_ago(2)
start_date = datetime(2020, 4, 10, 2, 0, 0)
end_date = exec_date + timedelta(seconds=30)
with create_session() as session:
for task in dag.tasks:
ti = TaskInstance(task=task, execution_date=exec_date, state="success")
ti.start_date = start_date
ti.end_date = end_date
session.add(ti)
with self.app.app_context():
mock_render_template.return_value = make_response("RESPONSE", 200)
url = 'gantt?dag_id={}&execution_date={}'.format(dag.dag_id, exec_date)
self.client.get(url, follow_redirects=True)
mock_render_template.assert_called_once_with(
'airflow/gantt.html',
base_date=mock.ANY, dag=mock.ANY,
data={
'taskNames': ['some_dummy_task_2'],
'tasks': [
{
'task_id': 'some_dummy_task_2',
'dag_id': 'ex_dag',
'execution_date': '2020-04-19T00:00:00+00:00',
'start_date': '2020-04-10T02:00:00+00:00', 'end_date': '2020-04-19T00:00:30+00:00',
'duration': None, 'state': 'success', 'try_number': 1,
'max_tries': 0, 'hostname': '', 'unixname': 'root',
'job_id': None, 'pool': 'default_pool', 'pool_slots': 1, 'queue': 'default',
'priority_weight': 1,
'operator': 'Dummy2TestOperator', 'queued_dttm': None,
'pid': None, 'executor_config': {},
'extraLinks': ['github', 'airflow']
}
], 'height': 50
},
demo_mode=False,
execution_date=mock.ANY, form=mock.ANY, root=mock.ANY, scheduler_job=mock.ANY
)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on issue #8505: [POC] Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617496233
I aadded catch_jinja_params contextmanager.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on issue #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-618243894
Great feature though
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on issue #8505: [POC] Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617510026
@kaxil We don't lose anything because we replace the method in the BaseView class with mock, and then mock calls the original. As a result, we have a wrapped method. If any class inherits from BaseView, it must use super () for mock to be called. Please note that `scheduler_job` key appears in the tests, which is added in another class, i.e. not VersionView.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil edited a comment on issue #8505: Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617461651
I recreated the Extra Links PR:
```python
@mock.patch('airflow.www.views.dagbag.get_dag')
@mock.patch('airflow.www.views.BaseView.render_template')
def test_extra_link_in_gantt_view(self, mock_render_template, get_dag_function):
from tests.test_utils.mock_operators import Dummy2TestOperator
dag = DAG('ex_dag', start_date=self.default_date)
task = Dummy2TestOperator(task_id="some_dummy_task_2", dag=dag)
get_dag_function.return_value = dag
exec_date = dates.days_ago(2)
start_date = datetime(2020, 4, 10, 2, 0, 0)
end_date = exec_date + timedelta(seconds=30)
with create_session() as session:
for task in dag.tasks:
ti = TaskInstance(task=task, execution_date=exec_date, state="success")
ti.start_date = start_date
ti.end_date = end_date
session.add(ti)
with self.app.app_context():
mock_render_template.return_value = make_response("RESPONSE", 200)
url = 'gantt?dag_id={}&execution_date={}'.format(dag.dag_id, exec_date)
self.client.get(url, follow_redirects=True)
mock_render_template.assert_called_once_with(
'airflow/gantt.html',
base_date=mock.ANY, dag=mock.ANY,
data={
'taskNames': ['some_dummy_task_2'],
'tasks': [
{
'task_id': 'some_dummy_task_2',
'dag_id': 'ex_dag',
'execution_date': '2020-04-19T00:00:00+00:00',
'start_date': '2020-04-10T02:00:00+00:00', 'end_date': '2020-04-19T00:00:30+00:00',
'duration': None, 'state': 'success', 'try_number': 1,
'max_tries': 0, 'hostname': '', 'unixname': 'root',
'job_id': None, 'pool': 'default_pool', 'pool_slots': 1, 'queue': 'default',
'priority_weight': 1,
'operator': 'Dummy2TestOperator', 'queued_dttm': None,
'pid': None, 'executor_config': {},
'extraLinks': ['github', 'airflow']
}
], 'height': 50
},
demo_mode=False,
execution_date=mock.ANY, form=mock.ANY, root=mock.ANY, scheduler_job=mock.ANY
)
```
arghh we will need to find a way to test individual args such that order of elements in list or duct doesn't matter.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil edited a comment on issue #8505: [POC] Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617505280
I like this, I tested it with:
```python
@mock.patch('airflow.www.views.dagbag.get_dag')
def test_extra_link_in_gantt_view(self, get_dag_function):
from tests.test_utils.mock_operators import Dummy2TestOperator
dag = DAG('dag', start_date=self.default_date)
Dummy2TestOperator(task_id="some_dummy_task_2", dag=dag)
get_dag_function.return_value = dag
exec_date = dates.days_ago(2)
start_date = datetime(2020, 4, 10, 2, 0, 0)
end_date = exec_date + timedelta(seconds=30)
with create_session() as session:
for task in dag.tasks:
ti = TaskInstance(task=task, execution_date=exec_date, state="success")
ti.start_date = start_date
ti.end_date = end_date
session.add(ti)
url = 'gantt?dag_id={}&execution_date={}'.format(self.dag.dag_id, exec_date)
with catch_jinja_params() as jinja_params:
self.client.get(url, follow_redirects=True)
self.assertEqual(jinja_params.template_name, 'airflow/gantt.html')
self.assertCountEqual(
jinja_params.template_params['data']['tasks'][0]['extraLinks'],
['airflow', 'github'])
```
and works fine. However, I have 1 question: Do we lose any Airflow related args/customizations because of `side_effect=BaseView.render_template` ??
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #8505: [POC] Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617470949
Converted to draft just so that we can discuss, I definitely like the approach :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-623195226
For future reference: https://flask.palletsprojects.com/en/1.1.x/signals/#subscribing-to-signals
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8505:
URL: https://github.com/apache/airflow/pull/8505#discussion_r412821804
##########
File path: tests/test_utils/asserts.py
##########
@@ -71,3 +72,33 @@ def assert_queries_count(expected_count, message_fmt=None):
"The current number is {current_count}."
message = message_fmt.format(current_count=result.count, expected_count=expected_count)
assert expected_count == result.count, message
+
+
+class CatchJinjaParmasResult:
+ def __init__(self):
+ self.template_name = None
+ self.template_params = None
+
+
+@contextmanager
+def catch_jinja_params():
+ """Catch jinja parameters."""
+ result = CatchJinjaParmasResult()
+
+ from flask_appbuilder import BaseView
+
+ with mock.patch(
+ 'flask_appbuilder.BaseView.render_template',
+ side_effect=BaseView.render_template,
+ autospec=True
+ ) as mock_render_template:
+ yield result
+
+ assert mock_render_template.call_count == 1, (
+ "It was expected that the BaseView.render_template method would only be called once. "
+ f"This was called {mock_render_template} times."
Review comment:
```suggestion
f"This was called {mock_render_template.call_count} times."
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-623191262
@potiuk @ashb @kaxil @turbaszek I have updated PR. Now I use flask.signals instead of mock.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil edited a comment on issue #8505: Add more precise tests for AirflowVersion
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-617461651
I recreated the Extra Links PR:
```python
@mock.patch('airflow.www.views.dagbag.get_dag')
@mock.patch('airflow.www.views.BaseView.render_template')
def test_extra_link_in_gantt_view(self, mock_render_template, get_dag_function):
from tests.test_utils.mock_operators import Dummy2TestOperator
dag = DAG('ex_dag', start_date=self.default_date)
task = Dummy2TestOperator(task_id="some_dummy_task_2", dag=dag)
get_dag_function.return_value = dag
exec_date = dates.days_ago(2)
start_date = datetime(2020, 4, 10, 2, 0, 0)
end_date = exec_date + timedelta(seconds=30)
with create_session() as session:
for task in dag.tasks:
ti = TaskInstance(task=task, execution_date=exec_date, state="success")
ti.start_date = start_date
ti.end_date = end_date
session.add(ti)
with self.app.app_context():
mock_render_template.return_value = make_response("RESPONSE", 200)
url = 'gantt?dag_id={}&execution_date={}'.format(dag.dag_id, exec_date)
self.client.get(url, follow_redirects=True)
mock_render_template.assert_called_once_with(
'airflow/gantt.html',
base_date=mock.ANY, dag=mock.ANY,
data={
'taskNames': ['some_dummy_task_2'],
'tasks': [
{
'task_id': 'some_dummy_task_2',
'dag_id': 'ex_dag',
'execution_date': '2020-04-19T00:00:00+00:00',
'start_date': '2020-04-10T02:00:00+00:00', 'end_date': '2020-04-19T00:00:30+00:00',
'duration': None, 'state': 'success', 'try_number': 1,
'max_tries': 0, 'hostname': '', 'unixname': 'root',
'job_id': None, 'pool': 'default_pool', 'pool_slots': 1, 'queue': 'default',
'priority_weight': 1,
'operator': 'Dummy2TestOperator', 'queued_dttm': None,
'pid': None, 'executor_config': {},
'extraLinks': ['github', 'airflow']
}
], 'height': 50
},
demo_mode=False,
execution_date=mock.ANY, form=mock.ANY, root=mock.ANY, scheduler_job=mock.ANY
)
```
arghh we will need to find a way to test individual args such that order of elements in list or dict doesn't matter.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on issue #8505: [POC] Add more precise tests (tests for jinja params) for AirflowVersion
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #8505:
URL: https://github.com/apache/airflow/pull/8505#issuecomment-618215731
Will look in more detail soon, but I don't think mocking is the best approach here - check out https://stackoverflow.com/a/40531281 or flask-testing module
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org