Fix Extra Links in Gannt View #8308
Conversation
Extra link didn't appear after changes in apache#8220 for Gantt View
kaxil
force-pushed
the
fix-extra-link-gantt-view
branch
from
3a66b26
to
9a25965
Compare
|
Should we add tests to avoid regression? |
|
I am not sure how to add a test for this. Any suggestions @ashb ? |
Was just typing that :D . We don't have any tests for Modal or at least I don't know, so any pointers would be helpful.
|
|
Whops, thanks Kaxil. The test to add would probably to ensure that the right extra_link name appears in the response. It's very poor test, but it's better than nothing. |
Added test |
Codecov Report
@@ Coverage Diff @@
## master #8308 +/- ##
==========================================
- Coverage 88.43% 6.27% -82.16%
==========================================
Files 940 940
Lines 45350 45358 +8
==========================================
- Hits 40105 2846 -37259
- Misses 5245 42512 +37267
Continue to review full report at Codecov.
|
Extra link didn't appear after changes in #8220 for Gantt View
Was able to use the @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. EDIT: The following worked @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']
) |
|
Waiting to hear what Ash thinks. The thing I like about your approach @mik-laj is like you said separates testing templates and flask views. On the other hand, the things we were doing until now is more like system test but I am leaning towards what you suggested because we were indeed searching for strings in the webserver source code :) |
|
In Django test there's a way of telling what template was rendered, and what the context was, without mocking (it also renders the template so any serious template errors are caught) Does flask have something similar? |
|
It is not available in Flask. |
Extra link didn't appear after changes in apache#8220 for Gantt View
Extra link didn't appear after changes in #8220 for Gantt View
Before:
After:
Make sure to mark the boxes below before creating PR: [x]
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.