PR: Run all tests, not stopping on first failure (Testing)

[juliangilbey]

Description of Changes

You might be interested in this patch (or not); it more closely resembles what I have done on Debian. Building on your improvements to conftest.py, this change is to attempt all the tests in the test suite on the first run (by removing the pytest -x option). The reason for this is that most failures I have observed are transient, so typically all pass on the second pytest run.

The other small change this patch makes because of the above change is to append the log files together rather than starting afresh each run; in this way, if a pytest run crashes in the middle (which I have often seen on non-amd64 architectures), passes from previous runs are not lost.

The one thing I have not done in this patch, but I think would be reasonable, is to reduce the number of pytest runs (run_tests.sh) in the .github/workflows/test-*.yml files from 8 down to, say, 4: if the tests have not all passed after 4 runs under this new scheme, then it is unlikely that they would in 4 further runs.

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:
@juliangilbey

[dalthviz]

Thanks @juliangilbey for looking into this! LGTM as it is but probably @ccordoba12 could have more ideas about this improvements.

[ccordoba12]

Thanks @juliangilbey for helping us with this! Besides the small comment I left for you below, these are my answers to your proposed changes:

this change is to attempt all the tests in the test suite on the first run

Ok, sounds reasonable.

The other small change this patch makes because of the above change is to append the log files together rather than starting afresh each run

Sounds good too. Actually, I didn't know about tee -a, that was the problem, so thanks a lot for that,

The one thing I have not done in this patch, but I think would be reasonable, is to reduce the number of pytest runs (run_tests.sh) in the .github/workflows/test-*.yml files from 8 down to, say, 4

I agree with this. Please do it as part of this PR.

[juliangilbey]

I've also added one more patch to conftest.py: with the --color option, the log file contains escape sequences immediately around PASSED and so on, so I've removed the spaces on either side of it in the regex.

[dalthviz]

Note: This will need a rebase after #18837 #18851 gets merged

[juliangilbey]

OK, my attempted fix to the regex included in this PR still doesn't work correctly on Linux. I hadn't used --color=yes on my setup, so hadn't appreciated the problem properly. On Linux, with this option, the output line from a successful test reads like this (output from od -t x1 -c):

0000000  73  70  79  64  65  72  2f  74  65  73  74  73  2f  74  65  73
          s   p   y   d   e   r   /   t   e   s   t   s   /   t   e   s
0000020  74  5f  64  6f  6e  74  5f  75  73  65  2e  70  79  3a  3a  74
          t   _   d   o   n   t   _   u   s   e   .   p   y   :   :   t
0000040  65  73  74  5f  64  6f  6e  74  5f  75  73  65  5b  69  73  69
          e   s   t   _   d   o   n   t   _   u   s   e   [   i   s   i
0000060  6e  73  74  61  6e  63  65  5c  5c  28  2e  2a  2c  2e  2a  73
          n   s   t   a   n   c   e   \   \   (   .   *   ,   .   *   s
0000100  74  72  5c  5c  29  2d  65  78  63  6c  75  64  65  5f  70  61
          t   r   \   \   )   -   e   x   c   l   u   d   e   _   p   a
0000120  74  74  65  72  6e  73  30  2d  44  6f  6e  27  74  20  75  73
          t   t   e   r   n   s   0   -   D   o   n   '   t       u   s
0000140  65  20  62  75  69  6c  74  69  6e  20  69  73  69  6e  73  74
          e       b   u   i   l   t   i   n       i   s   i   n   s   t
0000160  61  6e  63  65  28  29  20  66  75  6e  63  74  69  6f  6e  2c
          a   n   c   e   (   )       f   u   n   c   t   i   o   n   ,
0000200  75  73  65  20  73  70  79  64  65  72  2e  70  79  33  63  6f
          u   s   e       s   p   y   d   e   r   .   p   y   3   c   o
0000220  6d  70  61  74  2e  69  73  5f  74  65  78  74  5f  73  74  72
          m   p   a   t   .   i   s   _   t   e   x   t   _   s   t   r
0000240  69  6e  67  28  29  20  69  6e  73  74  65  61  64  5d  20  1b
          i   n   g   (   )       i   n   s   t   e   a   d   ]     033
0000260  5b  33  32  6d  50  41  53  53  45  44  1b  5b  30  6d  1b  5b
          [   3   2   m   P   A   S   S   E   D 033   [   0   m 033   [
0000300  33  33  6d  20  5b  20  32  35  25  5d  1b  5b  30  6d  0a
          3   3   m       [       2   5   %   ] 033   [   0   m  \n

which appears on screen as:

spyder/tests/test_dont_use.py::test_dont_use[isinstance\\(.*,.*str\\)-exclude_patterns0-Don't use builtin isinstance() function,use spyder.py3compat.is_text_string() instead] PASSED [ 25%]

with "PASSED" in green and "[ 25%]" in brown. My current attempt at fixing the regex, included in this PR, correctly identifies this line as a passing test, but calls the test spyder/tests/test_dont_use.py::test_dont_use[isinstance\\(.*,.*str\\)-exclude_patterns0-Don't use builtin isinstance() function,use spyder.py3compat.is_text_string() instead] \x1b[32m, which means it won't be skipped on the next run.

Two options to fix this:

• Remove the --color option from the Linux test and revert the change to conftest.py in this PR, though losing the color might be a shame
• Keep the --color option and change the regex line in conftest.py to read:

      test_re = re.compile(r'(spyder.*) [^ ]*(SKIPPED|PASSED|XFAIL)')

  (note no space at the end of the regex)

I wonder, though, whether I should take the change to conftest.py out of this PR and submit a fresh PR to fix this? If so, what would you like me to do about the conftest.py commit on this PR? I could do a git reset --hard HEAD^ on the branch and then force-push it, for example.

[dalthviz]

I think since all of these are improvements to the test suite keeping them all here is okay. I would said that the only thing to keep in mind is to make the changes related with the color in a single commit. Regarding the option to fix, I guess the second option is the way to go 👍

[juliangilbey]

Hi @dalthviz! OK, I'll go with the second option in this PR then. I've already made one color-related commit to try to fix conftest.py on this PR, but as I said, that doesn't work. I could push a further commit to this PR, or do a local second commit, then rebase onto the previous commit and do a force-push. Are you OK with the force-push approach?

[dalthviz]

The force-push approach sounds good to me 👍 Thanks for helping with this @juliangilbey !

[juliangilbey]

I hope this one does the job! It's somewhat hard for me to test it, but let me give it a go before you merge.

[dalthviz]

Just in case, the errors on the CI are unrelated to the changes here:

• The Windows installer build failure is due to cython 0.29.31 that was releases a couple of hours ago and most probably related to CDN Purging is broken, stale JSON responses are served instead pypi/warehouse#11949.

• The MacOS tests and installer build failures are going to be fixed with PR: Make the debugger work faster #18837 PR: Use Mac 12 to run tests on CIs and Mac 11 for the standalone app #18851

[juliangilbey]

OK, I've just tested my conftest.py patch with both color and no color, and it correctly handles passed tests on subsequent runs, so I'm happy with this PR.

[dalthviz]

Thanks @juliangilbey ! This LGTM 👍

[ccordoba12]

Thanks @juliangilbey!