[sage-devel] doctest quality and patchbots

['Martin R' via sage-devel]

Dear all,

in ticket https://trac.sagemath.org/ticket/26586 I just learned:

- The tests of individual functions within a single file are ALL
executed in the same environment.
- The tests of the individual functions within a single file are
executed in random order.

So I thought, I'd try `sage -t --randorder`. It turns out that there are
at least some failing tests in the combinat folder, see below.

I wonder how to continue with this. Please excuse that the following
questions seem unrelated to each other - however, they all deal with the
same problem at different levels.

1) would it be easy and desirable to make the patchbots run tests in random
order?
2) concerning https://trac.sagemath.org/ticket/26586, is it desirable to
define comparison for `CartesianProduct`? Or should I fix the doctest in
an ad-hoc manner (converting the `CartesianProduct` to tuples, and then
sort?
3) at first I thought that it is good practise to sort output whose order
is essentially random, but I was told not to do this. Is this advice good?
4) would it be good for `Poset.upper_covers` return a list (or iterator) of
tuples instead of a list (or iterator) of lists? Then one could turn the
list into a set easily.
5) would it be a good idea to have `__repr__` use a "sorted output" for
objects that are sets from a mathematical perspective? Of course, this may
break the user expectation what the first element of the displayed sequence
is, but on the other hand, it may make at least some output easier to
understand.

Best,

Martin

Failing tests after one run (note that there is at least one more which is
not robust but does not show up by chance here: poset_examples.py...)

sage -t src/sage/combinat/free_module.py # 4 doctests failed
sage -t src/sage/combinat/output.py # 8 doctests failed
sage -t src/sage/combinat/partition.py # 2 doctests failed
sage -t src/sage/combinat/k_tableau.py # 2 doctests failed
sage -t src/sage/combinat/root_system/weyl_characters.py # 1 doctest failed
sage -t src/sage/combinat/crystals/mv_polytopes.py # 1 doctest failed

[Jeroen Demeyer]

* The tests of individual functions within a single file are ALL
executed in the same environment.

Same "environment" in the sense of same Python process (there is one
process for each file to be tested). But global variables are reset for
each doctest block.

* The tests of the individual functions within a single file are
executed in random order.

This is false. Tests are run in order.

[Jeroen Demeyer]

Post by 'Martin R' via sage-devel
1) would it be easy and desirable to make the patchbots run tests in
random order?

Easy: yes
Desirable: no, it would create a lot of doctest failures

Post by 'Martin R' via sage-devel
2) concerning https://trac.sagemath.org/ticket/26586, is it desirable to
define comparison for `CartesianProduct`?

Absolutely. If you do that, you should open a new ticket.

Post by 'Martin R' via sage-devel
3) at first I thought that it is good practise to sort output whose
order is essentially random

I agree.

Post by 'Martin R' via sage-devel
5) would it be a good idea to have `__repr__` use a "sorted output" for
objects that are sets from a mathematical perspective?

I would say yes, but this wouldn't really solve the problem since the
function returns a list, which is not sorted.

It's important to note that such sorting should be restricted to
__repr__. For example, the method Poset.upper_covers() should not sort,
for 2 reasons:

(a) not all objects can be sorted, this is in particular a problem in
Python 3 (see graphs for a bad example)
(b) performance: most of the time, you don't care about ordering

[Simon King]

Hi Jeroen,

Post by Jeroen Demeyer

Post by 'Martin R' via sage-devel
1) would it be easy and desirable to make the patchbots run tests in
random order?

Easy: yes
Desirable: no, it would create a lot of doctest failures

... whose fixing is likely to make Sage run more stably. Afte all,
doctests that fail when being executed in a different order mean that
there are side-effects. And side-effects imply instability.

So, I'd tend to believe that it is desirable.

Best regards,
Simon

[Dima Pasechnik]

In such a setup, doctest framework will need to keep a record of the
particular order, and output it for each failure. otherwise things are not
reproducible...

Post by Simon King
Hi Jeroen,

Post by Jeroen Demeyer

Post by 'Martin R' via sage-devel
1) would it be easy and desirable to make the patchbots run tests in
random order?

Easy: yes
Desirable: no, it would create a lot of doctest failures

... whose fixing is likely to make Sage run more stably. Afte all,
doctests that fail when being executed in a different order mean that
there are side-effects. And side-effects imply instability.
So, I'd tend to believe that it is desirable.
Best regards,
Simon
--
You received this message because you are subscribed to the Google Groups
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an
Visit this group at https://groups.google.com/group/sage-devel.
For more options, visit https://groups.google.com/d/optout.

['Martin R' via sage-devel]

Post by Dima Pasechnik
In such a setup, doctest framework will need to keep a record of the

particular order, and output it for each failure. otherwise things are not
reproducible...

I think that this would be great, but at least in the cases I looked at,
the reason is that the reason for failure is different order of elements or
the like.

Should I open a meta-ticket?

Martin

[E. Madison Bray]

Post by Simon King
Hi Jeroen,

Post by Jeroen Demeyer

Post by 'Martin R' via sage-devel
1) would it be easy and desirable to make the patchbots run tests in
random order?

Easy: yes
Desirable: no, it would create a lot of doctest failures

... whose fixing is likely to make Sage run more stably. Afte all,
doctests that fail when being executed in a different order mean that
there are side-effects. And side-effects imply instability.
So, I'd tend to believe that it is desirable.

+1 There are several tests which, when run in an unusual order, result
in random failures. This is obviously a failure of test isolation if
nothing else, and such cases *should* be rooted out and fixed. There
are quite a lot of them though--I've stumbled on them many times but
rarely bothered with it myself.

But as a matter of principle I agree with the suggestion of running
tests in random order and fixing any problems that arise. Perhaps for
now there would be too many failures that it shouldn't be the default.
But I would definitely encourage anyone who wants to to try to fix
some of those failures...

[Jeroen Demeyer]

Post by E. Madison Bray
+1 There are several tests which, when run in an unusual order, result
in random failures. This is obviously a failure of test isolation if
nothing else, and such cases *should* be rooted out and fixed.

It's not a failure of "test isolation" if nobody ever claimed that tests
*are* isolated. The only way to really have test isolation is to run a
separate process for each test. We already do that for separate files,
but not for individual tests.

[E. Madison Bray]

Post by Jeroen Demeyer

Post by E. Madison Bray
+1 There are several tests which, when run in an unusual order, result
in random failures. This is obviously a failure of test isolation if
nothing else, and such cases *should* be rooted out and fixed.

It's not a failure of "test isolation" if nobody ever claimed that tests
*are* isolated. The only way to really have test isolation is to run a
separate process for each test. We already do that for separate files,
but not for individual tests.

But I sometimes get failures in tests depending on which order the
*files* were run in. That's what I'm talking about. You'd think that
shouldn't happen but it can: especially since some tests do things
like write to or read resources from the user's .sage directory. By
test isolation I don't just mean isolation of tests from each other,
but also from their environment (through which tests *can* become
entangled and subject to non-local interactions :)

['Martin R' via sage-devel]

Actually, I find it more problematic that sage -t --long may pass (which is
what the patchbots check), but sage -t fails (which is what most users
running the tests probably see)

Martin

Post by E. Madison Bray

Post by Jeroen Demeyer

Post by E. Madison Bray
+1 There are several tests which, when run in an unusual order, result
in random failures. This is obviously a failure of test isolation if
nothing else, and such cases *should* be rooted out and fixed.

It's not a failure of "test isolation" if nobody ever claimed that tests
*are* isolated. The only way to really have test isolation is to run a
separate process for each test. We already do that for separate files,
but not for individual tests.

But I sometimes get failures in tests depending on which order the
*files* were run in. That's what I'm talking about. You'd think that
shouldn't happen but it can: especially since some tests do things
like write to or read resources from the user's .sage directory. By
test isolation I don't just mean isolation of tests from each other,
but also from their environment (through which tests *can* become
entangled and subject to non-local interactions :)

[Jeroen Demeyer]

Post by E. Madison Bray
But I sometimes get failures in tests depending on which order the
*files* were run in. That's what I'm talking about.

But that's really unusual (and unrelated to --randorder). Do you have
*any* concrete example of such a failure?

[E. Madison Bray]

Post by Jeroen Demeyer

Post by E. Madison Bray
But I sometimes get failures in tests depending on which order the
*files* were run in. That's what I'm talking about.

But that's really unusual (and unrelated to --randorder).

I guess I misunderstood what --randorder does. I thought it would
just randomize the order in which files were tested but apparently it
randomizes doctests within a file (and if so, what's the point of
that?) I think from docstring to docstring at least there should be
better test isolation.

Post by Jeroen Demeyer
Do you have
*any* concrete example of such a failure?

Not at the moment but I've had it before: For example, a test might
pass if I ran `./sage -t -a`, but fail if I just ran `./sage -t
src/sage/some_subpackage`. Usually when this has come up I didn't
bother to investigate further.