You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Deng Xiaodong <xd...@gmail.com> on 2018/10/13 11:51:09 UTC
Something May Be Wrong with the Travis CI Tests
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
Re: Something May Be Wrong with the Travis CI Tests
Posted by "Driesprong, Fokko" <fo...@driesprong.frl>.
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...@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...@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
> > >
> >
>
Re: Something May Be Wrong with the Travis CI Tests
Posted by Deng Xiaodong <xd...@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...@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
> >
>
Re: Something May Be Wrong with the Travis CI Tests
Posted by "Driesprong, Fokko" <fo...@driesprong.frl>.
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...@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
>