If I understand correctly, using `pytest -k` might be less work and more generalized than a swag of custom makers, unless it entails a lot of re-naming things. The work to add markers might be easier if they can be applied to entire classes of tests, although what I've mostly seen with `pytest` is a functional pattern rather than classes in tests. For more about that, see the note about using pytest fixtures vs. class setup/teardown at https://docs.pytest.org/en/latest/xunit_setup.html
With regard to "slow" and https://github.com/apache/airflow/pull/6876, it was motivated by one test that uses moto mocking for AWS batch services. In particular, it has a mock batch job that actually runs a container and the user of the mock has no control over how the job transitions from various job states (with associated status). For example, the `pytest` durations are an order of magnitude longer for this test than all others (see below stdout from a PR branch of mine). So, during dev-test cycles, once this test is coded and working as expected, it helps to either temporarily mark it with `pytest.mark.skip` or to permanently mark it with a custom marker (e.g. `pytest.mark.slow`) and then use the `pytest -m 'not slow'` to run all the faster tests. It's no big deal, I can live without it, it's just a convenience. 13.77s call tests/providers/amazon/aws/hooks/test_batch_waiters.py::test_aws_batch_job_waiting 0.23s setup tests/providers/amazon/aws/hooks/test_batch_waiters.py::test_aws_batch_job_waiting 0.11s call tests/providers/amazon/aws/hooks/test_batch_client.py::TestAwsBatchClient::test_poll_job_complete_raises_for_max_retries 0.09s call tests/providers/amazon/aws/hooks/test_batch_waiters.py::TestAwsBatchWaiters::test_wait_for_job_raises_for_client_error 0.01s call tests/providers/amazon/aws/hooks/test_batch_client.py::TestAwsBatchClientDelays::test_exponential_delay_03 On Fri, Dec 27, 2019 at 11:39 AM Tomasz Urbaszek < tomasz.urbas...@polidea.com> wrote: > The suggestion of using -k flag is really interesting. It will require a > lot of changes but adding > marks will require the same effort. However, I think that using a marker > is more explicit and > easier to spot. > > Regarding "slow test" marker, I did a quick calculation of tests execution > times (execution and setups): > > Above 1s : 16.53%, time together: 18.29m > Above 2s : 8.13%, time together: 14.43m > Above 3s : 4.14%, time together: 11.54m > Above 4s : 2.94%, time together: 10.3m > Above 5s : 2.2%, time together: 9.3m > Above 10s : 0.89%, time together: 6.51m > Above 20s : 0.47%, time together: 4.53m > Above 60s : 0.0%, time together: 0.0m > > > Total time of the example build: 28m. I am not sure when a test is "slow". > Moreover, I think there could be > a difference in times between local environment (where developer will > decide to use such marker) and > the CI environment, thus resulting in a potential inconsistency. > > T. > > On Fri, Dec 27, 2019 at 7:58 PM Darren Weber <dweber.consult...@gmail.com> > wrote: > > > > > > > Consider all the options for filtering tests: > > - http://doc.pytest.org/en/latest/example/markers.html > > > > The `pytest -k` filters are very useful. Provide guidelines on how to > > name things so that `pytest -k` can be used to filter categories of > tests. > > Use markers for tests that might be the exception to the rule within a > > module or class of tests (avoid the overhead of marking all the tests > with > > all the markers). Consider using classes that contain all the same > marker > > (but a module and/or class name could serve this purpose too). > > > > -- Darren > > > > PS, I stumbled in this after creating > > https://github.com/apache/airflow/pull/6876 to mark a slow test > > > > On 2019/12/27 11:30:09, Tomasz Urbaszek <turbas...@apache.org> wrote: > > > +1 for integrations and backends, it's a good start ;) > > > > > > T. > > > > > > On Fri, Dec 27, 2019 at 12:16 PM Jarek Potiuk < > jarek.pot...@polidea.com> > > > wrote: > > > > > > > Since I am going to start working on it soon - I'd love to get some > > > > opinions :). > > > > > > > > J. > > > > > > > > On Mon, Dec 23, 2019 at 11:13 AM Jarek Potiuk < > > jarek.pot...@polidea.com> > > > > wrote: > > > > > > > > > I have a concrete proposal that we can start with. It's not a final > > set > > > > of > > > > > markers we might want to have but one that we can start with and > > make an > > > > > immediate use of. > > > > > > > > > > I would like to adapt our tests to be immediately usable in Breeze > > (and > > > > > tied with it) and follow this approach: > > > > > > > > > > *Proposed Breeze changes:* > > > > > > > > > > - `./breeze` by default will start only the main > 'airflow-testing' > > > > > image. This way no huge resource usage will be needed when > breeze > > is > > > > > started by default > > > > > - './breeze --all-integrations` will start all dependent images > > (so we > > > > > will be able to run all tests) > > > > > - './breeze --integrations [kubernetes,cassandra,mongo, > > > > > rabbitmq,redis,openldap,kerberos] - you will be able to choose > > which > > > > > integrations you want to start > > > > > - When you run `breeze --backend postgres` it will only start > > postgres > > > > > not mysql and the other way round. > > > > > > > > > > *Proposed Pytest marks:* > > > > > > > > > > - > > > > > > > > > > > > pytest.mark.integrations('kubernetes'),pytest.mark.integrations('cassandra'),..... > > > > > - pytest,mark.backends("postgres"), > pytest,mark.backends("mysql"), > > > > > pytest.mark.backends("sqlite") > > > > > > > > > > It's very easy to add custom switches to pytest and auto-detect > what > > is > > > > > the default setting based on environment variables for example. We > > could > > > > > follow > > > > > > > > > > > > https://docs.pytest.org/en/latest/example/markers.html#custom-marker-and-command-line-option-to-control-test-runs > > > > > . > > > > > > > > > > *Proposed Pytest behaviour:* > > > > > > > > > > - `pytest` -> in Breeze will run all tests that are applicable > > within > > > > > the current environment: > > > > > - it will only run non-marked tests by default, applicable > with > > > > > current selected backend > > > > > - when (for example) you stared cassandra is added it will > > > > > additionally run pytest.mark.integrations('cassandra') > > > > > - `pytest` in local environment by default will only run > > non-marked > > > > > tests > > > > > - `pytest --integrations [kubernetes, ....]` will only run the > > > > > integration tests selected (will convert the switch into the > > > > corresponding > > > > > markers (as explained in the example above) > > > > > - `pytest --backends [postgres| mysql | sqlite] will only run > the > > > > > specific tests that use postgres/mysql/sqlite specific tests > > > > > > > > > > *What we will achieve by that:* > > > > > > > > > > - lower resource usage by Breeze by default (while allowing to > run > > > > > most of the tests) > > > > > - easy selection of integration(s) we want to test > > > > > - easy way to run all tests to reproduce CI run > > > > > - capability of running just 'pytest' and testing (as fast as > > > > > possible) all the tests that are applicable in your environment > > (if > > > > you > > > > > want to be extra-sure everything works - for example during > > > > refactoring) > > > > > - in the future we might be able to optimise CI and run smaller > > set of > > > > > tests for postgres/mysql/sqlite 'only' cases - optimising the > time > > > > for CI > > > > > builds. > > > > > > > > > > > > > > > If I will get a general "OK" from community for that - I can make a > > set > > > > of > > > > > incremental changes to breeze (as I continue working on prod image) > > and > > > > add > > > > > those capabilities to Breeze. > > > > > > > > > > J. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Dec 18, 2019 at 1:10 AM Kamil Breguła < > > kamil.breg...@polidea.com > > > > > > > > > > wrote: > > > > > > > > > >> It is worth adding that we currently use test marking in the > > project. > > > > For > > > > >> this purpose, we use the prefix "_system.py" in the file name. > > > > >> Unit tests: > > > > >> > > > > >> > > > > > > > https://github.com/apache/airflow/blob/master/tests/operators/test_gcs_to_gcs.py > > > > >> System tests: > > > > >> > > > > >> > > > > > > > https://github.com/apache/airflow/blob/master/tests/operators/test_gcs_to_gcs_operator_system.py > > > > >> Elsewhere, a special directory structure is used. > > > > >> Unit tests: > > > > >> https://github.com/apache/airflow/tree/master/tests/kubernetes > > > > >> Integration tests: > > > > >> > > > > > > > https://github.com/apache/airflow/tree/master/tests/integration/kubernetes > > > > >> > > > > >> This will allow us to limit e.g. mocking in system tests. > > > > >> This seems to be a clearer solution because it clearly separates > > each > > > > type > > > > >> of test. If we add markers, they may not be noticed when making > > changes > > > > >> and > > > > >> review. The file name is immediately visible. > > > > >> Recently I dealt with such a case that system tests included > > mocking, > > > > >> which > > > > >> by definition did not work. > > > > >> > > > > >> > > > > > > > https://github.com/apache/airflow/commit/11262c6d42c4612890a6eec71783e0a6d5b22c17 > > > > >> > > > > >> > > > > >> On Tue, Dec 10, 2019 at 2:22 PM Jarek Potiuk < > > jarek.pot...@polidea.com> > > > > >> wrote: > > > > >> > > > > >> > I am all-in for markers. > > > > >> > > > > > >> > I think we should start with small set of useful markers, which > > should > > > > >> have > > > > >> > a useful purpose from the beginning and implement them first - > to > > > > learn > > > > >> how > > > > >> > useful they are (before we decide on full set of markers). > > > > >> > Otherwise maintaining those markers will become a fruitless > > "chore" > > > > and > > > > >> it > > > > >> > might be abandoned. > > > > >> > > > > > >> > So my proposal is to agree the first top cases we want to handle > > with > > > > >> > markers and then define/apply the markers accordingly: > > > > >> > > > > > >> > Those are my three top priorities (from most important to > least): > > > > >> > > > > > >> > - Splitting out the Integration tests (and updating Breeze) > so > > that > > > > >> you > > > > >> > choose which integration you start when you start Breeze > rather > > > > than > > > > >> > start > > > > >> > them all. > > > > >> > - DB separation so that we do not repeat non-DB tests on all > > > > >> Databases. > > > > >> > - Proper separation of Kubernetes tests (They are now > filtered > > out > > > > >> based > > > > >> > on skipif/env variables. > > > > >> > > > > > >> > > > > > >> > J. > > > > >> > > > > > >> > > > > > >> > On Tue, Dec 10, 2019 at 1:32 PM Tomasz Urbaszek < > > > > >> > tomasz.urbas...@polidea.com> > > > > >> > wrote: > > > > >> > > > > > >> > > Hi everyone, > > > > >> > > > > > > >> > > Since we run our tests using pytest we are able to use test > > markers > > > > >> [1]. > > > > >> > > Using them will give > > > > >> > > use some useful things: > > > > >> > > - additional information of test type (ex. when used for > system > > > > test) > > > > >> > > - easy way to select test by types (ex. pytest -v -m "not > > system") > > > > >> > > - way to split our test suite in more effective way (no need > to > > run > > > > >> all > > > > >> > > tests on 3 backends) > > > > >> > > > > > > >> > > I would like to discuss what "official" marks would we like to > > use. > > > > >> As a > > > > >> > > base I would suggests > > > > >> > > to mark tests as: > > > > >> > > - system - tests that need the outside world to be successful > > (ex. > > > > GCP > > > > >> > > system tests) > > > > >> > > - db[postgres, sqlite, mysql] - tests that require database to > > be > > > > >> > > successful, in other words, > > > > >> > > tests that create some db side effects > > > > >> > > - integration - tests that requires some additional resources > > like > > > > >> > > Cassandra or Kubernetes > > > > >> > > > > > > >> > > All other, unmarked tests would be treated as "pure" meaning > > that > > > > they > > > > >> > have > > > > >> > > no side effects > > > > >> > > (at least on database level). > > > > >> > > > > > > >> > > What do you think about this? Does anyone have some experience > > with > > > > >> using > > > > >> > > markers in > > > > >> > > such a big project? > > > > >> > > > > > > >> > > [1] http://doc.pytest.org/en/latest/example/markers.html > > > > >> > > > > > > >> > > > > > > >> > > Bests, > > > > >> > > Tomek Urbaszek > > > > >> > > > > > > >> > > > > > >> > > > > > >> > -- > > > > >> > > > > > >> > Jarek Potiuk > > > > >> > Polidea <https://www.polidea.com/> | Principal Software > Engineer > > > > >> > > > > > >> > M: +48 660 796 129 <+48660796129> > > > > >> > [image: Polidea] <https://www.polidea.com/> > > > > >> > > > > > >> > > > > > > > > > > > > > > > -- > > > > > > > > > > Jarek Potiuk > > > > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > > > > > > > M: +48 660 796 129 <+48660796129> > > > > > [image: Polidea] <https://www.polidea.com/> > > > > > > > > > > > > > > > > > > -- > > > > > > > > Jarek Potiuk > > > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > > > > > M: +48 660 796 129 <+48660796129> > > > > [image: Polidea] <https://www.polidea.com/> > > > > > > > > > > > > -- > > Tomasz Urbaszek > Polidea <https://www.polidea.com/> | Software Engineer > > M: +48 505 628 493 <+48505628493> > E: tomasz.urbas...@polidea.com <tomasz.urbasz...@polidea.com> > > Unique Tech > Check out our projects! <https://www.polidea.com/our-work> > -- Darren L. Weber, Ph.D. http://psdlw.users.sourceforge.net/ http://psdlw.users.sourceforge.net/wordpress/
