TST: display reason of skipping by default with pytest #10946

jorisvandenbossche · 2018-04-10T12:00:36Z

Reference Issues/PRs

See discussion in #10835 (comment)
Actually doing this change, will make it easier to see how it looks on Travis to decide if this is too verbose or not.

With this change, pytest will show the reason that a test is skipped by default. This can be especially useful to not be surprised why your doctests are skipping because you don't have numpy 1.14.

lesteve · 2018-04-10T13:02:08Z

You need to figure out why the Python 3.4 is failing. No suggestions off the top of my head I am afraid ...

About 20 lines in the Python 3.6 build (with numpy >= 1.14)
https://travis-ci.org/scikit-learn/scikit-learn/jobs/364572375#L2844

About 100 lines in the Python 2.7 build (with numpy < 1.14)
https://travis-ci.org/scikit-learn/scikit-learn/jobs/364572373#L3044

I am fine with merging this. It is useful in the numpy >= 1.14 case to avoid suprises. You could also argue it is useful in general to be reminded about skipped tests that we may want to fix at one point.

jorisvandenbossche · 2018-04-10T14:42:00Z

Hmm, one thing I can think of that the failing build seems to use a much older version of pytest (3.0.5 vs 3.5.0). I think the reason for this is that conda does not have recent packages anymore for Python 3.4 (both main channel as conda-forge).

lesteve · 2018-04-10T15:32:04Z

build_tools/travis/install.sh

@@ -61,6 +61,8 @@ if [[ "$DISTRIB" == "conda" ]]; then

    conda create -n testenv --yes $TO_INSTALL
    source activate testenv
+    conda install pip --yes


pip is installed in any conda environment (at least by default) so you don't need this line

can you only install pytest if $PYTHON_VERSION == "3.4" so that we can drop this special case once we stop supporting Python 3.4?

can you use pip install --upgrade --no-deps pytest. Just a bit paranoid with pip install -U that upgrades all the dependencies ...

Yes, I was planning to do those things, just wanted to make sure it was actually fixing it before diving putting time in it :-)

Will clean up now

jnothman · 2018-04-10T22:44:58Z

build_tools/travis/install.sh

@@ -62,6 +62,11 @@ if [[ "$DISTRIB" == "conda" ]]; then
    conda create -n testenv --yes $TO_INSTALL
    source activate testenv

+    # for python 3.4, conda does not have recent pytest packages
+    if [[ "$PYTHON_VERSION" == "3.4" ]]; then


If it's possible to use another conda channel, that might be better than overwriting a conda install with pip

It's not in defaults, and not in conda-forge, and from a quick search on anaconda.org it does not seem they have their own channel

jnothman · 2018-04-10T22:45:39Z

setup.cfg

@@ -7,6 +7,7 @@ test = pytest
 addopts =
    --doctest-modules
    --disable-pytest-warnings
+    -r s


Could we please use the long form options?

I changed 's' to 'skipped' (that apparently works), but for "-r" I don't see any long format in the pytest help

jnothman · 2018-04-11T08:01:11Z

I'll take your word on -r.
LGTM

lesteve · 2018-04-11T08:22:06Z

build_tools/travis/install.sh

@@ -62,6 +62,11 @@ if [[ "$DISTRIB" == "conda" ]]; then
    conda create -n testenv --yes $TO_INSTALL
    source activate testenv

+    # for python 3.4, conda does not have recent pytest packages
+    if [[ "$PYTHON_VERSION" == "3.4" ]]; then
+        pip install --upgrade --no-deps pytest


If I had to guess how to quickly fix the error, I would do pip install pytest==3.5.

I guess I was a bit naive about pip install --upgrade --no-deps. It probably does not install dependencies and I was hoping it would only avoid upgrading the dependencies.

Or if six is the only missing dependency, install that as well.

But actually, somewhat below we also do pip install coverage codecov, so maybe we don't need to care to much about not installing dependencies with pip?

lesteve · 2018-04-11T08:59:42Z

Travis passes, I am going to merge this one. Thanks @jorisvandenbossche for making the world a slightly less surprising place ;-).

jorisvandenbossche · 2018-04-11T09:04:04Z

Thanks!

rth · 2018-04-22T21:01:35Z

I think this should have been -r s as in earlier commits not -r skipped as the latest version,

$ pytest --help
[...]
  -r chars              show extra test summary info as specified by chars
                        (f)ailed, (E)error, (s)skipped, (x)failed, (X)passed,
                        (p)passed, (P)passed with output, (a)all except pP.
                        Warnings are displayed at all times except when
                        --disable-warnings is set

currently passing tests are also included in the test summary in the end
(since ("p" in "skipped") is True), As a result the failing tracebacks are not and the end of the CI log but somewhere in the middle, which is hard to find for contributors.

TST: display reason of skipping by default with pytest

09f2922

jorisvandenbossche mentioned this pull request Apr 10, 2018

[MRG + 1] Run doctests only on numpy 1.14 and fix them with numpy 1.14 formatting changes #10835

Merged

check if installing latest pytest fixes it

fc4e900

lesteve reviewed Apr 10, 2018

View reviewed changes

clean-up update command

0654b98

jorisvandenbossche force-pushed the pytest-show-skip-reason branch from a63d26b to 0654b98 Compare April 10, 2018 16:03

jnothman reviewed Apr 10, 2018

View reviewed changes

s -> skipped

be9e392

lesteve reviewed Apr 11, 2018

View reviewed changes

try pip install pytest==3.5

6ec81b9

lesteve merged commit 312f640 into scikit-learn:master Apr 11, 2018

jorisvandenbossche deleted the pytest-show-skip-reason branch April 11, 2018 09:03

rth mentioned this pull request Apr 22, 2018

MAINT: don't display passed tests in pytest summary #11013

Merged

lesteve mentioned this pull request Jul 19, 2023

CI Add summary about failures and errors in most builds #26847

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST: display reason of skipping by default with pytest #10946

TST: display reason of skipping by default with pytest #10946

jorisvandenbossche commented Apr 10, 2018

lesteve commented Apr 10, 2018

jorisvandenbossche commented Apr 10, 2018

lesteve Apr 10, 2018 •

edited

Loading

jorisvandenbossche Apr 10, 2018 •

edited

Loading

jnothman Apr 10, 2018

jorisvandenbossche Apr 11, 2018

jnothman Apr 10, 2018

jorisvandenbossche Apr 11, 2018

jnothman commented Apr 11, 2018

lesteve Apr 11, 2018 •

edited

Loading

jorisvandenbossche Apr 11, 2018

lesteve commented Apr 11, 2018 •

edited

Loading

jorisvandenbossche commented Apr 11, 2018

rth commented Apr 22, 2018 •

edited

Loading

TST: display reason of skipping by default with pytest #10946

TST: display reason of skipping by default with pytest #10946

Conversation

jorisvandenbossche commented Apr 10, 2018

Reference Issues/PRs

lesteve commented Apr 10, 2018

jorisvandenbossche commented Apr 10, 2018

lesteve Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

jorisvandenbossche Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

jnothman Apr 10, 2018

Choose a reason for hiding this comment

jorisvandenbossche Apr 11, 2018

Choose a reason for hiding this comment

jnothman Apr 10, 2018

Choose a reason for hiding this comment

jorisvandenbossche Apr 11, 2018

Choose a reason for hiding this comment

jnothman commented Apr 11, 2018

lesteve Apr 11, 2018 • edited Loading

Choose a reason for hiding this comment

jorisvandenbossche Apr 11, 2018

Choose a reason for hiding this comment

lesteve commented Apr 11, 2018 • edited Loading

jorisvandenbossche commented Apr 11, 2018

rth commented Apr 22, 2018 • edited Loading

lesteve Apr 10, 2018 •

edited

Loading

jorisvandenbossche Apr 10, 2018 •

edited

Loading

lesteve Apr 11, 2018 •

edited

Loading

lesteve commented Apr 11, 2018 •

edited

Loading

rth commented Apr 22, 2018 •

edited

Loading