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/05/07 15:52:46 UTC

[GitHub] [airflow] uranusjr opened a new pull request #15719: Refactor tests/www/test_views.py (Part 2/2)

uranusjr opened a new pull request #15719:
URL: https://github.com/apache/airflow/pull/15719


   Following up #15666
   
   This converts the rest of `tests/www/test_views.py` into `tests/www/views/` and use pytest fixtures to manage them.
   
   (Run time numbers are different from #15666 since I’m working on another machine.)
   
   ```
   (master) $ ./breeze tests -- tests/www/test_views.py tests/www/views/
   ========== 246 passed, 2 skipped, 10 warnings in 170.95s (0:02:50) ==========
   
   (tests-www-refactor) $ ./breeze tests -- tests/www/views/
   ========== 252 passed, 2 skipped, 10 warnings in 164.77s (0:02:44) ==========
   ```
   
   Not much difference since the tests were already sharing resources well. But the refactoring is still very worthwhile IMOby virtue of breaking done the three convoluted monster classes in `test_views.py`.
   
   The test count increased slightly because I split a few tests in `test_views.py` into smaller ones with `parametrize`. They were doing too much in one tests and did not correctly manage test setup very well.
   
   There are still some possible performance boosts left. The `app` fixture in `tests/ww/views/` is currently initialised per-module because some tests modify `app.dag_bag`. But not all modules do, and those that don’t can share an `app` instance instead. But that would make the test setup more complicated (if we incorrectly re-use `app` when we shouldn’t the tests could become stateful and incorrect) so I opted to not do it.
   
   After this I will work on increasing test coverage of `airflow/www/views.py` as suggested in #15525.
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   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).
   


-- 
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 #15719: Refactor tests/www/test_views.py (Part 2/2)

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #15719:
URL: https://github.com/apache/airflow/pull/15719#issuecomment-838287429


   Tests are failing :(


-- 
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 #15719: Refactor tests/www/test_views.py (Part 2/2)

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #15719:
URL: https://github.com/apache/airflow/pull/15719#issuecomment-837578992


   @uranusjr There are some conflicts -- can you fix them please


-- 
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 merged pull request #15719: Refactor tests/www/test_views.py (Part 2/2)

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #15719:
URL: https://github.com/apache/airflow/pull/15719


   


-- 
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