Hi XD, This is a very valid point. I think most of the Operators are still good, since the PythonOperator is used quite a lot also in other tests, but we should re-enable these tests as well like you mention. Nevertheless it might be that the tests are outdated because they aren't updated. Thanks for addressing this and picking it up.
This is also a nice opportunity for people who want to get involved into contributing to Airflow. Cheers, Fokko Op za 13 okt. 2018 om 14:48 schreef Deng Xiaodong <xd.den...@gmail.com>: > Hi Fokko, > > I have tried your idea. You are correct: after prepend the filename with > "test_", the CI test failed as we want ( > > https://travis-ci.org/XD-DENG/incubator-airflow/builds/440983339?utm_source=github_status&utm_medium=notification > ). > It DOES relate to the test discovery. > > We need to tackle this issue to make sure these tests really work (by > prepending the test file names with "test_"). > > But my concern is that some of these tests were never really run, and their > corresponding operators/hooks/sensors may be very "unhealthy" (only in > folder "tests/operators", there are 9 test scripts which were not named > correctly, i.e., never really run). We can fix the tests itself > quite easily, but fixing the potential "accumulated" issues in these > corresponding operators/hooks/sensors may make this a big ticket to work > on. > > Please let me know what you think. > (I will start from DockerOperator first though). > > > XD > > On Sat, Oct 13, 2018 at 8:02 PM Driesprong, Fokko <fo...@driesprong.frl> > wrote: > > > Hi XD, > > > > Very good point. I was looking into this recently, but since time is a > > limited matter, I did not really dig into it. It has to do with the test > > discovery. The python_operator does not match the given pattern test*.py: > > > > > https://docs.python.org/3/library/unittest.html#cmdoption-unittest-discover-p > > > > Could you try to prepend the filename with test_. For example, > > test_python_operator.py? > > > > Cheers, Fokko > > > > Op za 13 okt. 2018 om 13:51 schreef Deng Xiaodong <xd.den...@gmail.com>: > > > > > Hi folks, especially our committers, > > > > > > Something may be wrong with our Travis CI tests, unless I > > > misunderstood/missed something. > > > > > > I'm checking *DockerOperator*, and some implementations inside are not > > > making sense to me. But no CI tests ever failed due to it. When I check > > the > > > log of the historical Travis CI, surprisingly, I found the test of > > > DockerOperator never really run (you search any one of the recent > Travis > > > log). > > > > > > To prove this, I forked the latest master branch and tried to add "self > > > .assertTrue(1 == 0)" into the code of > tests/operators/docker_operator.py > > > < > > > > > > https://github.com/XD-DENG/incubator-airflow/commit/2d6f47202349aa75b8d3e8e1631a285d2d75f1e7#diff-17e0452f4ce967751edfa767d46ae0ce > > > > > > > and tests/operators/python_operator.py > > > < > > > > > > https://github.com/XD-DENG/incubator-airflow/commit/d7e4205f2f25dc2ea29356e4f43543f9b0bca963#diff-b5351e876d48957e2b64da5c16b0bd60 > > > >, > > > which would for sure fail the tests. However, and as I suspected, the > > > Travis CI passed ( > > > https://github.com/XD-DENG/incubator-airflow/commits/patch-6). This > > means > > > these two tests were never invoked during the Travis CI, and I believe > > > these two are not the only tests affected. > > > > > > May anyone take a look into this? If I did misunderstand/miss > something, > > > kindly let me know. > > > > > > Many thanks! > > > > > > XD > > > > > >
