CI/TST: Making ci/run_tests.sh fail if one of the steps fail #24075

datapythonista · 2018-12-03T18:47:14Z

Looks like when simplifying the running of the tests in the CI (#23924), I missed the -e in the bash header. And that makes the ci/run_tests.sh exit with status code 0, even if the calls to pytests fail.

This left the CI in green, even when tests fail for the last 3 days (sorry about that). I think nothing is broken.

This PR fixes the problem.

CC: @pandas-dev/pandas-core

gfyoung · 2018-12-03T19:04:55Z

Seems reasonable to me. Wondering though if we should double check by adding a failing test just to test that this fails as expected?

datapythonista · 2018-12-03T19:08:51Z

I tested locally. I'd personally merge this asap, as the test of all PRs are failing silently (unless you check the logs). And we'll see quickly if this works as expected as soon as other PR tests fail.

But I surely can break something here and test if you think it's worth the delay.

gfyoung · 2018-12-03T20:00:23Z

But I surely can break something here and test if you think it's worth the delay.

Fair enough. I think we all can be vigilant to notice if it doesn't work. 🙂

gfyoung

Let's merge on green.

TomAugspurger · 2018-12-03T20:49:36Z

Any idea what the failure is from @datapythonista? https://travis-ci.org/pandas-dev/pandas/jobs/462954286#L1845

$ ci/run_tests.sh
============================= test session starts ==============================
platform linux2 -- Python 2.7.15, pytest-4.0.1, py-1.7.0, pluggy-0.8.0
rootdir: /home/travis/build/pandas-dev/pandas, inifile: setup.cfg
plugins: xdist-1.24.1, forked-0.2, hypothesis-3.68.0
gw0 [0]
scheduling tests via LoadScheduling
 generated xml file: /home/travis/build/pandas-dev/pandas/test-data-single.xml -
======================== no tests ran in 84.95 seconds =========================
The command "ci/run_tests.sh" exited with 5.

Both of those failures are the slow / single tests, right?

datapythonista · 2018-12-03T21:27:50Z

Looks like when no tests are found with a -m pattern (the case for single and slow), the pytest process exits with status code 5. I'm checking what's the best option to fix it.

TomAugspurger · 2018-12-03T21:36:53Z

I see. Do we ever want to run single and slow though? I would have thought that it'd be single or slow.

edit: no that's not quite right... sorry.

TomAugspurger · 2018-12-03T21:40:27Z

So to run all the SLOW jobs, we need

-n 1 -m slow and single

and then

-n 2 -m slow and not single

And the problem is that -m slow and single is empty? As a workaround, can we add a dummy test with both those marks?

codecov · 2018-12-03T21:41:56Z

Codecov Report

Merging #24075 into master will increase coverage by 49.86%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #24075       +/-   ##
===========================================
+ Coverage   42.38%   92.25%   +49.86%     
===========================================
  Files         161      161               
  Lines       51701    51701               
===========================================
+ Hits        21914    47696    +25782     
+ Misses      29787     4005    -25782

Flag	Coverage Δ
#multiple	`90.65% <ø> (?)`
#single	`42.38% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/computation/pytables.py	`92.37% <0%> (+0.3%)`	⬆️
pandas/io/pytables.py	`92.3% <0%> (+0.92%)`	⬆️
pandas/util/_test_decorators.py	`93.24% <0%> (+4.05%)`	⬆️
pandas/compat/__init__.py	`58.36% <0%> (+8.17%)`	⬆️
pandas/core/config_init.py	`99.24% <0%> (+9.84%)`	⬆️
pandas/core/reshape/util.py	`100% <0%> (+11.53%)`	⬆️
pandas/compat/numpy/__init__.py	`92.85% <0%> (+14.28%)`	⬆️
pandas/core/computation/common.py	`85.71% <0%> (+14.28%)`	⬆️
pandas/core/api.py	`100% <0%> (+14.81%)`	⬆️
pandas/core/indexes/api.py	`99% <0%> (+14.85%)`	⬆️
... and 119 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08395af...0a67310. Read the comment docs.

codecov · 2018-12-03T21:41:57Z

Codecov Report

Merging #24075 into master will increase coverage by 49.7%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #24075      +/-   ##
==========================================
+ Coverage   42.38%   92.09%   +49.7%     
==========================================
  Files         161      161              
  Lines       51701    52102     +401     
==========================================
+ Hits        21914    47983   +26069     
+ Misses      29787     4119   -25668

Flag	Coverage Δ
#multiple	`90.66% <ø> (?)`
#single	`42.81% <ø> (+0.43%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/computation/pytables.py	`92.37% <0%> (+0.3%)`	⬆️
pandas/io/pytables.py	`92.3% <0%> (+0.92%)`	⬆️
pandas/util/_test_decorators.py	`93.24% <0%> (+4.05%)`	⬆️
pandas/compat/__init__.py	`58.36% <0%> (+8.17%)`	⬆️
pandas/core/config_init.py	`99.24% <0%> (+9.84%)`	⬆️
pandas/core/reshape/util.py	`100% <0%> (+11.53%)`	⬆️
pandas/core/dtypes/dtypes.py	`89.82% <0%> (+13.66%)`	⬆️
pandas/compat/numpy/__init__.py	`92.85% <0%> (+14.28%)`	⬆️
pandas/core/computation/common.py	`85.71% <0%> (+14.28%)`	⬆️
pandas/core/api.py	`100% <0%> (+14.81%)`	⬆️
... and 119 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08395af...07375c7. Read the comment docs.

datapythonista · 2018-12-03T21:44:45Z

That's correct @TomAugspurger, looks like it's a common pytest issue. The pytest devs propose the workaround I pushed. Not the nicest code, but this should work for now, and I'll try to find a cleaner approach for that script later (I don't like calling pytest twice, will try to avoid that).

TomAugspurger · 2018-12-03T21:47:51Z

LGTM.

…

On Mon, Dec 3, 2018 at 3:44 PM Marc Garcia ***@***.***> wrote: That's correct @TomAugspurger <https://github.com/TomAugspurger>, looks like it's a common pytest issue. The pytest devs propose the workaround I pushed. Not the nicest code, but this should work for now, and I'll try to find a cleaner approach for that script later (I don't like calling pytest twice, will try to avoid that). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24075 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIh6DxgCCo7bAnCFs4m-IAOKvVRJMks5u1ZtSgaJpZM4Y_L32> .

datapythonista · 2018-12-04T00:02:03Z

Can't really understand the errors in travis. They are surely unrelated, they don't seem to happen in master, and I can't reproduce them locally. @TomAugspurger have you seen this error before?

_____________________ TestTSPlot.test_is_error_nozeroindex _____________________
[gw0] linux -- Python 3.6.6 /home/travis/miniconda3/envs/pandas-dev/bin/python
self = <pandas.tests.plotting.test_datetimelike.TestTSPlot object at 0x7fca6c1875c0>
    def test_is_error_nozeroindex(self):
        # GH11858
        i = np.array([1, 2, 3])
        a = DataFrame(i, index=i)
>       _check_plot_works(a.plot, xerr=a)
pandas/tests/plotting/test_datetimelike.py:89: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pandas/tests/plotting/test_datetimelike.py:1562: in _check_plot_works
    pickle.dump(fig, path)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
m = <bound method AffineBase.transform of <matplotlib.transforms.BlendedAffine2D object at 0x7fca6c567240>>
    def _pickle_method(m):
        """Handle pickling issues with class instance."""
>       if m.im_self is None:
E       AttributeError: 'function' object has no attribute 'im_self'
../../../miniconda3/envs/pandas-dev/lib/python3.6/site-packages/statsmodels/graphics/functional.py:32: AttributeError

h-vetinari · 2018-12-04T00:29:11Z

I've seen them locally sometimes, but never figured out why. Seems like a weird matplotlib dependency on something.

datapythonista · 2018-12-04T00:55:15Z

That's weird, not sure why matplotlib or statmodels should fail randomly.

In any case, all green now. I'll let someone else review it before merging, as I made some changes since last reviews.

jreback · 2018-12-04T02:09:44Z

ci/run_tests.sh

-    pytest -m "$TYPE_PATTERN$PATTERN" -n $NUM_JOBS -s --strict --durations=10 --junitxml=test-data-$TYPE.xml $TEST_ARGS $COVERAGE pandas
+    PYTEST_CMD="pytest -m \"$TYPE_PATTERN$PATTERN\" -n $NUM_JOBS -s --strict --durations=10 --junitxml=test-data-$TYPE.xml $TEST_ARGS $COVERAGE pandas"
+    echo $PYTEST_CMD
+    # if no tests are found (the case of "single and slow"), pytest exits with code 5, and would make the script fail, if not for the below code


if no tests are found it is an error

we don't have tests for the builds when we filter by "slow", when we execute the "single" call here. So, all our slow builds will fail.

ok thanks
even more explicit comments here would help then

but can be more another pass

I'll try to simplify all this to a single call with the --dist option, and if that works, this or the loop won't be needed, and things will be very simple here

…dev#24075) * Making ci/run_tests.sh fail if one of the steps fail * Fixing the pytest error when no tests are found for single and slow * Fixing error when uploading the coverage

Making ci/run_tests.sh fail if one of the steps fail

4a76e85

datapythonista added Testing pandas testing functions or related to the test suite CI Continuous Integration labels Dec 3, 2018

gfyoung approved these changes Dec 3, 2018

View reviewed changes

Fixing the pytest error when no tests are found for single and slow

0a67310

Fixing error when uploading the coverage

07375c7

jreback added this to the 0.24.0 milestone Dec 4, 2018

jreback requested changes Dec 4, 2018

View reviewed changes

jreback merged commit 00f5aba into pandas-dev:master Dec 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI/TST: Making ci/run_tests.sh fail if one of the steps fail #24075

CI/TST: Making ci/run_tests.sh fail if one of the steps fail #24075

datapythonista commented Dec 3, 2018 •

edited

Loading

gfyoung commented Dec 3, 2018

datapythonista commented Dec 3, 2018

gfyoung commented Dec 3, 2018

gfyoung left a comment

TomAugspurger commented Dec 3, 2018

datapythonista commented Dec 3, 2018

TomAugspurger commented Dec 3, 2018 •

edited

Loading

TomAugspurger commented Dec 3, 2018

codecov bot commented Dec 3, 2018

codecov bot commented Dec 3, 2018 •

edited

Loading

datapythonista commented Dec 3, 2018

TomAugspurger commented Dec 3, 2018 via email

datapythonista commented Dec 4, 2018

h-vetinari commented Dec 4, 2018

datapythonista commented Dec 4, 2018

jreback Dec 4, 2018

datapythonista Dec 4, 2018

jreback Dec 4, 2018

datapythonista Dec 4, 2018

jreback Dec 4, 2018

CI/TST: Making ci/run_tests.sh fail if one of the steps fail #24075

CI/TST: Making ci/run_tests.sh fail if one of the steps fail #24075

Conversation

datapythonista commented Dec 3, 2018 • edited Loading

gfyoung commented Dec 3, 2018

datapythonista commented Dec 3, 2018

gfyoung commented Dec 3, 2018

gfyoung left a comment

Choose a reason for hiding this comment

TomAugspurger commented Dec 3, 2018

datapythonista commented Dec 3, 2018

TomAugspurger commented Dec 3, 2018 • edited Loading

TomAugspurger commented Dec 3, 2018

codecov bot commented Dec 3, 2018

Codecov Report

codecov bot commented Dec 3, 2018 • edited Loading

Codecov Report

datapythonista commented Dec 3, 2018

TomAugspurger commented Dec 3, 2018 via email

datapythonista commented Dec 4, 2018

h-vetinari commented Dec 4, 2018

datapythonista commented Dec 4, 2018

jreback Dec 4, 2018

Choose a reason for hiding this comment

datapythonista Dec 4, 2018

Choose a reason for hiding this comment

jreback Dec 4, 2018

Choose a reason for hiding this comment

datapythonista Dec 4, 2018

Choose a reason for hiding this comment

jreback Dec 4, 2018

Choose a reason for hiding this comment

datapythonista commented Dec 3, 2018 •

edited

Loading

TomAugspurger commented Dec 3, 2018 •

edited

Loading

codecov bot commented Dec 3, 2018 •

edited

Loading