Add missing test modules in our unit test suite #35442
Comments
|
cc: @eladkal |
This test aimed to make sure that we add tests when we add modules ... But it seems it's been broken for a while and generally it expected no tests at all and never complained (Who tests the tests?) The root cause was generous (pun intended) use of generators - simply speaking when generator gets iterated over, the second time it gets iterated, it returns ... nothing ... These list are not really huge so using generators here has been largely premature optmisation that makes it also difficult to debug, for now I converted it to use lists - debugging of lists is way easier too. We have the list of tests now that we should add and they are captured in the issue apache#35442
This test aimed to make sure that we add tests when we add modules ... But it seems it's been broken for a while and generally it expected no tests at all and never complained (Who tests the tests?) The root cause was generous (pun intended) use of generators - simply speaking when generator gets iterated over, the second time it gets iterated, it returns ... nothing ... These list are not really huge so using generators here has been largely premature optmisation that makes it also difficult to debug, for now I converted it to use lists - debugging of lists is way easier too. We have the list of tests now that we should add and they are captured in the issue #35442
Since our test to detect missing tests was broken, lack of the test_file_transfer.py (among many others) has not been detected before. This PR adds a simple test that mocks out ObjectStoragePath and tests that they are initialized properly and that execute method executes `copy` on the source Object poth (but also does not call copy on the target one). Part of apache#35442 Also unblocks apache#35241 which fails becasuse Pytest fails when it sees no tests collected when `Providers[common.io]'` TEST_TYPE is used.
|
Also related to #35127 |
|
cc: @ephraimbuddy -> this might guide people on where to add tests to cover whole modules that are not "targeted" for tests (they might still be accidentally tested by other modules but they do not have dedicated module tests) - this might help people to make decision on where to add some tests. Also this is a nice list of "todos" |
potiuk
changed the title
Since our test to detect missing tests was broken, lack of the test_file_transfer.py (among many others) has not been detected before. This PR adds a simple test that mocks out ObjectStoragePath and tests that they are initialized properly and that execute method executes `copy` on the source Object poth (but also does not call copy on the target one). Part of #35442 Also unblocks #35241 which fails becasuse Pytest fails when it sees no tests collected when `Providers[common.io]'` TEST_TYPE is used.
I think how to exclude case when the original module |
|
And just add here previous investigation about missing tests #28459 (comment), non all of them actually missing:
|
|
And |
In similar cases we used to have deprecated classes list and we removed it from the output |
Absolutely - I think there are many false-negatives there, so removal them by finding and automated way and improving the test case is absolutely legitimate way of cleaning some of those. PRs are most welcome :). I just revived the test really quickly - also in order to not allow more of new ones creep in, but I think we should definitely remove some of those from the list by making the test "smarter"
Oh absolutely - this is why I added my comment #35442 (comment) and updated #35127 with the comment that these two are connected. I see that as two sides of the same coing - eventually they should converge - the missing coverage should quite closely show similar list as the one generated here. Currently they are far-off - for the reasons you explained, but also because some of the module's code are tested by some other tests. - but this is precisely I think what should guide us in making the test smarter - eventually they should show the same result.
Yeah - why don't we automate that in the test then? I think some of them can be easily automatically detected. |
|
If you don't mind I've split this list by providers |
Not at all ! Very helpful I think :) |
|
Super useful with codecov links BTW! Good idea. |
…#35443) This test aimed to make sure that we add tests when we add modules ... But it seems it's been broken for a while and generally it expected no tests at all and never complained (Who tests the tests?) The root cause was generous (pun intended) use of generators - simply speaking when generator gets iterated over, the second time it gets iterated, it returns ... nothing ... These list are not really huge so using generators here has been largely premature optmisation that makes it also difficult to debug, for now I converted it to use lists - debugging of lists is way easier too. We have the list of tests now that we should add and they are captured in the issue apache#35442
Since our test to detect missing tests was broken, lack of the test_file_transfer.py (among many others) has not been detected before. This PR adds a simple test that mocks out ObjectStoragePath and tests that they are initialized properly and that execute method executes `copy` on the source Object poth (but also does not call copy on the target one). Part of apache#35442 Also unblocks apache#35241 which fails becasuse Pytest fails when it sees no tests collected when `Providers[common.io]'` TEST_TYPE is used.
|
Hi! I found that the classes present in the following modules are already covered with tests, but they are unchecked in the issue's first comment: tests/providers/amazon/aws/operators/test_emr.py If it is possible, it would be nice if anyone could check them, so that who is looking to contribute can find missing tests more easily. Thanks! CC: @potiuk @Taragolis |
Hi @Taragolis ! Thanks for your help. I guess I was not so clear, but the corresponding code of each of that non-existing files is covered in other test files, and that is why I put those ones on the list. For example, one test file from the above list that does not exist is
So the |
|
There is pretty simple rule there if providers module placed into the Original check was broken for a while (1-2 years or so), so we have about 100 modules which do not follow this one. |
See
test_providers_modules_should_have_testsintests/always/test_project_structure.pybut those tests are missing currently:Provider amazon
codecov
Provider apache.cassandra
codecov
Provider apache.drill
codecov
Provider apache.druid
codecov
Provider apache.hdfs
codecov
Provider apache.hive
codecov
Provider apache.kafka
codecov
Provider celery
codecov
Provider cncf.kubernetes
codecov
Provider common.io
codecov
Provider databricks
codecov
Provider dbt.cloud
codecov
Provider docker
codecov
Provider elasticsearch
codecov
Provider google
codecov
Provider microsoft.azure
codecov
Provider mongo
codecov
Provider openlineage
codecov
Provider presto
codecov
Provider redis
codecov
Provider slack
codecov
Provider snowflake
codecov
Provider trino
codecov
Committer
The text was updated successfully, but these errors were encountered: