Re: Something May Be Wrong with the Travis CI Tests

2018-10-13 Thread Deng Xiaodong
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_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 
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 :
>
> > 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

2018-10-13 Thread Driesprong, Fokko
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 :

> 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
>


Something May Be Wrong with the Travis CI Tests

2018-10-13 Thread Deng Xiaodong
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

 and tests/operators/python_operator.py
,
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