Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace unittests test cases by pure pytest [Wave-1] #26831

Merged
merged 1 commit into from Oct 10, 2022

Conversation

Taragolis
Copy link
Contributor

@Taragolis Taragolis commented Oct 2, 2022

A lot of tests still uses unittest.TestCase class as base method for test classes as well as internals of this class. May be it a good time to start convert tests to use native pytest functionality.

Total number of modules which uses unittest.TestCase

❯ grep -rl 'TestCase):' ./tests | wc -l
579

Total number of modules which uses unittest.TestCase

❯ grep -rl 'TestCase):' ./tests | cut -d"/" -f3 | sort | uniq -c | sort -nr
    428 providers
     36 charts
     23 utils
     21 cli
     10 ti_deps
     10 api_connexion
      7 operators
      6 sensors
      6 executors
      6 core
      5 www
      5 always
      4 models
      3 api
      2 task
      2 kubernetes
      1 security
      1 plugins
      1 macros
      1 hooks
      1 dag_processing

Total number of providers tests submodules which uses unittest.TestCase (by submodules)

Note: "apache", "microsoft", "common" has own sub-packages which contains separate providers

❯ grep -rl 'TestCase):' ./tests/providers/ | cut -d"/" -f5 | sort | uniq -c | sort -nr
    146 google
    108 amazon
     33 apache
     29 microsoft
      6 databricks
      4 redis
      4 qubole
      4 mysql
      4 alibaba
      3 yandex
      3 trino
      3 tableau
      3 oracle
      3 jenkins
      3 http
      3 hashicorp
      3 docker
      3 atlassian
      3 arangodb
      3 airbyte
      2 vertica
      2 telegram
      2 sqlite
      2 snowflake
      2 sftp
      2 segment
      2 salesforce
      2 presto
      2 postgres
      2 opsgenie
      2 neo4j
      2 mongo
      2 jdbc
      2 influxdb
      2 imap
      2 grpc
      2 ftp
      2 exasol
      2 discord
      2 dingding
      2 datadog
      2 common
      2 cncf
      2 asana
      1 ssh
      1 singularity
      1 sendgrid
      1 samba
      1 papermill
      1 openfaas
      1 elasticsearch
      1 cloudant
      1 celery

This PR focus on only simple changes in tests exclude providers and charts:

  • Remove base class unittest.TestCase
  • setUp method -> setup_method
  • tearDown method -> teardown_method
  • setUpClass classmethod -> setup_class
  • tearDownClass -> teardown_class
  • parameterized.expand decorator -> pytest.mark.parametrize
  • self.assertRaises context manager -> pytest.raises

Result after changes

❯ grep -rl 'TestCase' ./tests | cut -d"/" -f3 | sort | uniq -c | sort -nr
    428 providers
     36 charts
      4 operators
      2 utils
      2 models
      2 cli
      2 always
      1 www
      1 test_utils
      1 sensors
      1 security
      1 executors
      1 core

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:CLI area:plugins labels Oct 2, 2022
Comment on lines 168 to 172
def test_does_send_stats_using_dogstatsd_when_statsd_and_dogstatsd_both_on(self):
# ToDo: Figure out why it identical to test_does_send_stats_using_dogstatsd_when_dogstatsd_on
self.dogstatsd.incr("empty_key")
self.dogstatsd_client.increment.assert_called_once_with(
metric='empty_key', sample_rate=1, tags=[], value=1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just add todo to figure out later why implementation same as test_does_send_stats_using_dogstatsd_when_dogstatsd_on

@Taragolis Taragolis force-pushed the replace-unittest-by-pytest-wave-1 branch from 3c1a0dd to 614d3dd Compare October 3, 2022 07:36
Comment on lines 179 to +181
def test_markdown_none(self):
rendered = self.attr_renderer["python_callable"](None)
assert "" == rendered
rendered = self.attr_renderer["doc_md"](None)
assert rendered is None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also "fix" this test seems like it just copy of test_python_callable_none but name of the test more related to doc_md attribute

@Taragolis Taragolis force-pushed the replace-unittest-by-pytest-wave-1 branch from 614d3dd to d9988a1 Compare October 3, 2022 15:08
@Taragolis Taragolis marked this pull request as ready for review October 3, 2022 16:43
@Taragolis Taragolis force-pushed the replace-unittest-by-pytest-wave-1 branch from d9988a1 to 4c1f0d1 Compare October 3, 2022 17:40
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn’t read everything, but this seems straightforward enough from the 10-ish files I sampled. This should be good as long as CI passes.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

COOL!

@potiuk potiuk merged commit 1c7c976 into apache:main Oct 10, 2022
@eladkal eladkal mentioned this pull request Nov 2, 2022
ephraimbuddy pushed a commit that referenced this pull request Nov 10, 2022
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Nov 10, 2022
@ephraimbuddy ephraimbuddy added this to the Airflow 2.4.3 milestone Nov 10, 2022
ephraimbuddy pushed a commit that referenced this pull request Nov 10, 2022
@Taragolis Taragolis deleted the replace-unittest-by-pytest-wave-1 branch January 14, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:CLI area:plugins changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants