Skip to content

Allow automatic re-running of failed tests#401

Merged
mshinwell merged 5 commits into
ocaml:trunkfrom
dra27:testsuite-retries
Jan 26, 2016
Merged

Allow automatic re-running of failed tests#401
mshinwell merged 5 commits into
ocaml:trunkfrom
dra27:testsuite-retries

Conversation

@dra27

@dra27 dra27 commented Jan 6, 2016

Copy link
Copy Markdown
Member

Various tests are documented as occasionally failing (e.g. lib-threads) - some commands, especially in the Windows ports, can fail randomly and simply need to be re-run.
This patch adds a MAX_RETRIES setting to testsuite/Makefile which permits automatically re-running an entire directory of tests up MAX_RETRIES times if any of the tests fail. The summary-generator is updated to display the union of all the test results - i.e. if a test passes in any of the runs, then it is deemed to have passed.
May hopefully provide a slightly more reliable (or quiet) testsuite mode for Windows CI builds.
I tried an earlier version for re-running the specific test that failed, rather than the entire directory, but that very quickly became a very invasive change...

@dra27 dra27 force-pushed the testsuite-retries branch from 713d5ee to 857af1a Compare January 6, 2016 20:52
Comment thread testsuite/Makefile

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice factorization, thanks.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not precisely identical to the previous one (needing all-basic to include tests/basic means that all-tool would include tests/tool if it existed, etc.) but I figured that was OK!

@dra27 dra27 force-pushed the testsuite-retries branch from 857af1a to 481f9bc Compare January 11, 2016 09:57
Comment thread testsuite/Makefile Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd much rather have MAX_RETRIES set in config/Makefile, rather than doing auto-config on the fly.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops - at the very least, it should have been above the include, to allow config/Makefile to override the automatic default! I'll move it to the Windows Makefiles and update configure appropriately (if it's going "globally", I guess MAX_TESTSUITE_DIR_RETRIES might be a better name?)

@dra27 dra27 force-pushed the testsuite-retries branch 2 times, most recently from cab2661 to a47930e Compare January 13, 2016 16:15
@dra27

dra27 commented Jan 13, 2016

Copy link
Copy Markdown
Member Author

MAX_RETRIES moved to config/Makefile.* (or generated by configure) and renamed to MAX_TESTSUITE_DIR_RETRIES

@gasche

gasche commented Jan 13, 2016

Copy link
Copy Markdown
Member

I'm ok for merging; @damiendoligez?

@dra27 dra27 force-pushed the testsuite-retries branch from a47930e to ddd50fc Compare January 15, 2016 22:44
@dra27

dra27 commented Jan 17, 2016

Copy link
Copy Markdown
Member Author

Forgive me for adding additional commits here, but the work wants to be based on the refactored awk script from here, rather than trunk.
The first extra commit is an amendment to restore --traditional functionality to the script - I noticed, for example, on my Ubuntu 14.04 LTS that the default installation of awk does not support GNU's switch extension, so I converted it to use a series of ifs.
The remaining 3 commits address two problems I've hit while running this testsuite myriad times on various configurations:

  1. Silence doesn't necessarily indicate that tests are passing - an error in an earlier version of this awk script in the very first commit meant that unexpected errors were being ignored the summary - so it looked like a 100% pass rate, but in fact the number of passing tests was (significantly) lower than it should have been.
  2. Skipping tests silently means we forget about them! For example, lib-threads/sockets.ml should have been re-enabled for Windows when the two PRs causing it to fail were fixed (I've fixed that in Various test-suite fixes for Windows #366). While displaying the details of tests which are skipped won't automatically cause them to be reconsidered, it at least reduces the chances of forgetting about them completely!
    So these remaining three commits alter the tests so that " => skipped" is displayed for each and every test which is skipped (including the more subtle cases when BYTECODE_ONLY=true). It therefore displays not only a summary of the skipped tests but also the total number of tests which were considered - one could start embedding this either in testsuite/makefiles/summarize.awk or in a specific file and display an error if the number of tests run doesn't match. Indeed, what I'm heading towards here is the possibility that the _log file itself can have almost have a reference file for a given platform/configuration.

@dra27 dra27 force-pushed the testsuite-retries branch 3 times, most recently from 971236d to e818abe Compare January 17, 2016 11:36
@dra27

dra27 commented Jan 17, 2016

Copy link
Copy Markdown
Member Author

You can see the Unix output on lines 3369-3378 of the Travis build, but here for comparison is the testsuite output from an x86 VS 2015 build of dra27/ocaml/visual-studio (which is a merging of all my various pending pull requests)

Summary:
  534 tests passed
   24 tests skipped
    0 tests failed
    0 unexpected errors
  558 tests considered

List of skipped tests:
    tests/lib-threads/signal2.ml
    tests/lib-systhreads
    tests/lib-dynlink-csharp
    tests/asmcomp
    tests/runtime-errors/stackoverflow.native
    tests/lib-bigarray-2
    tests/callback

@dra27 dra27 force-pushed the testsuite-retries branch from e818abe to d33a304 Compare January 17, 2016 19:07
@alainfrisch

Copy link
Copy Markdown
Contributor

some commands, especially in the Windows ports, can fail randomly and simply need to be re-run.

Do you have an idea about the cause of these "random" failures?

@dra27

dra27 commented Jan 18, 2016

Copy link
Copy Markdown
Member Author

They've included:

  • Random failures from the mt tool - this seems to be a known issue with old versions of mt, and I've not seen it with Visual Studio 2010+, I don't think
  • Threading tests: I think there are notes that these can fail randomly on any platform?
    For example, the Visual Studio 2015 x64 reference I've been working to in the last couple of days failed the first time on lib-threads:
Running tests from 'tests/lib-threads' ...
 ... testing 'bank.ml': ocamlc ocamlopt => passed
 ... testing 'beat.ml': ocamlc ocamlopt => passed
 ... testing 'bufchan.ml': ocamlc ocamlopt => passed
 ... testing 'close.ml': ocamlc ocamlopt => passed
 ... testing 'fileio.ml': ocamlc ocamlopt => passed
 ... testing 'pr4466.ml': ocamlc ocamloptThread 2 killed on uncaught exception Unix.Unix_error(56, "recv", "")^M
Thread 3 killed on uncaught exception Unix.Unix_error(56, "recv", "")^M
Fatal error: exception Unix.Unix_error(56, "send", "")^M
/cygdrive/c/Users/DRA/Documents/ocaml-x64/testsuite/makefiles/Makefile.several:97: recipe for target 'run-file' failed
make[4]: *** [run-file] Error 2
 => failed
 ... testing 'pr5325.ml': ocamlc ocamlopt => passed
 ... testing 'prodcons.ml': ocamlc ocamlopt => passed
 ... testing 'prodcons2.ml': ocamlc ocamlopt => passed
 ... testing 'sieve.ml': ocamlc ocamlopt => passed
 ... testing 'signal.ml': ocamlc ocamlopt => passed
 ... testing 'signal2.ml': => skipped
 ... testing 'sockets.ml': ocamlc ocamloptFatal error: exception Unix.Unix_error(56, "read", "")^M
/cygdrive/c/Users/DRA/Documents/ocaml-x64/testsuite/makefiles/Makefile.several:97: recipe for target 'run-file' failed
make[4]: *** [run-file] Error 2
 => failed
 ... testing 'socketsbuf.ml': ocamlc ocamlopt => passed
 ... testing 'swapchan.ml': ocamlc ocamlopt => passed
 ... testing 'tls.ml': ocamlc ocamlopt => passed
 ... testing 'torture.ml': ocamlc ocamlopt => passed

but the automatic re-run had passes with no issues. Note that my patch does clean out the test before retrying, so it shouldn't ever be the case that a re-run becomes a systemic requirement of a test.

@dra27

dra27 commented Jan 18, 2016

Copy link
Copy Markdown
Member Author

I forgot the principal culprit:

  • Cygwin fork! i.e. bash failing...

@alainfrisch

Copy link
Copy Markdown
Contributor

Thanks for the info. I was guessing that Cygwin might be suspected, but is that a documented fact?

@dra27

dra27 commented Jan 18, 2016

Copy link
Copy Markdown
Member Author

Well, it's not really a bug - it's just that running make (especially in parallel - I saw it a lot working on OPAM - though I haven't been parallelising anything here), there's a lot of fork-ing going on and Cygwin's emulation of fork is incredibly complicated. As far as I can tell, it's essentially caused by momentarily running out of resources.

@damiendoligez damiendoligez added this to the 4.03.0 milestone Jan 22, 2016
@damiendoligez

Copy link
Copy Markdown
Member

I confirm that I've seen random failures for the threads tests on all platforms (but with different frequencies).

@dra27

dra27 commented Jan 25, 2016

Copy link
Copy Markdown
Member Author

Rebased on to trunk with a small adjustment to "Display details of all skipped tests" (now e6d6afc) to correctly implement the disabling of tests/asmcomp for msvc64 in 031d4ac

@dra27

dra27 commented Jan 26, 2016

Copy link
Copy Markdown
Member Author

Note that this will need rebasing again once #366 is merged - the conflict resolution isn't totally obvious (see https://github.com/dra27/ocaml/tree/visual-studio)

@mshinwell

Copy link
Copy Markdown
Contributor

@dra27 I have merged #366, please deal with this one as appropriate.

dra27 added 3 commits January 26, 2016 14:46
MAX_TESTSUITE_DIR_RETRIES (which defaults to 1 for the native Windows
ports and 0 for all other platforms) causes the `all` and `list` targets
to re-run any given test directory an additional number of times in
order to eliminate random failures during tests.
switch is a GNU extension - not supported, for example, by the default
installation of awk on Ubuntu.
Some Makefile's displayed "=> skipped" lines; some Makefile's displayed
a summary line including the word skipped; most Makefile's are simply
silent if tests are skipped.

Silent obviously doesn't discern between "skipped" and "broken"!

Patch alters all existing to display an explicit "=> skipped" for any
test which is skipped (including ocamlopt tests if
`BYTECODE_ONLY=true`).

The summary is updated to include:

 - The number of skipped tests, and their names (if all tests are
   skipped in a directory then just the directory name is displayed)
 - The total number of tests considered - which should therefore be the
   same on all platforms in all configurations
dra27 added 2 commits January 26, 2016 14:47
David R Allsopp -- pedantically eschewing lazy plural(s) since 1994.
Testsuite directories should now display " => skipped" for each test,
rather than simply displaying nothing.
@dra27 dra27 force-pushed the testsuite-retries branch from eeed995 to 771d1bd Compare January 26, 2016 16:40
@dra27

dra27 commented Jan 26, 2016

Copy link
Copy Markdown
Member Author

That all seems fine (x86 and x64 testsuite pass with Visual Studio 2015) - Travis should hopefully agree shortly.

mshinwell added a commit that referenced this pull request Jan 26, 2016
GPR#401: Allow automatic re-running of failed tests
@mshinwell mshinwell merged commit b1d549c into ocaml:trunk Jan 26, 2016
@dra27 dra27 deleted the testsuite-retries branch February 8, 2016 09:23
poechsel added a commit to poechsel/ocaml that referenced this pull request Apr 12, 2021
chambart pushed a commit to chambart/ocaml-1 that referenced this pull request Jan 4, 2022
Optimize bits_of_float / float_of_bits with memory operands
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Co-authored-by: Thibaut <thibaut.mattio@gmail.com>
@dra27 dra27 mentioned this pull request Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants