From 89424b41ef62430a49012c5c57b1979f29029505 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 4 Nov 2016 08:50:12 +0200 Subject: Implement concurrent_runner --- build2/test/script/runner.cxx | 409 ++++++++++++++++++++++++++++++++- build2/test/script/script | 7 + tests/buildfile | 2 +- tests/test/buildfile | 7 + tests/test/script/buildfile | 7 + tests/test/script/driver.cxx | 76 ++++++ tests/test/script/testscript | 42 ++++ unit-tests/test/script/lexer/buildfile | 2 +- 8 files changed, 548 insertions(+), 4 deletions(-) create mode 100644 tests/test/buildfile create mode 100644 tests/test/script/buildfile create mode 100644 tests/test/script/driver.cxx create mode 100644 tests/test/script/testscript diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx index 3a5eb76..051d0f8 100644 --- a/build2/test/script/runner.cxx +++ b/build2/test/script/runner.cxx @@ -2,9 +2,18 @@ // copyright : Copyright (c) 2014-2016 Code Synthesis Ltd // license : MIT; see accompanying LICENSE file +#ifndef _WIN32 +# include // WIFEXITED(), WEXITSTATUS() +#endif + +#include // cerr + +#include // auto_rm + #include using namespace std; +using namespace butl; namespace build2 { @@ -29,11 +38,231 @@ namespace build2 print_test (r, t); } + // Test command output cache. The usage is as follows: + // + // 1. Call wopen() to open the stream in write mode and register the file + // for auto-removal by dtor. + // + // 2. Pass the file descriptor to the test command process ctor to + // redirect it's output to the cache file. + // + // 3. Close the stream. + // + // 4. Call ropen() to open the file in read mode to match the content to + // a pattern, cancel the file removal if the match fails (so the + // output is available for troubleshooting). + // + class stream_cache: public ifdstream + { + public: + using path_type = butl::path; + + stream_cache (const char* n): name (n) {} + + // Open stream for writing. Return the file descriptor. Must not be + // called multiple times. + // + int + wopen () + { + // Otherwise compiler gets confused with basic_ios::fail(). + // + using build2::fail; + + assert (!exists ()); + + try + { + // @@ Later it will make sense to create the file in the + // test-specific temporary directory. + // + path = path_type::temp_path ("build2"); + } + catch (const system_error& e) + { + fail << "unable to create temporary file: " << e.what (); + } + + try + { + open (path, out | trunc); + cleanup = auto_rm (path); + } + catch (const io_error& e) + { + fail << "unable to write " << path << ": " << e.what (); + } + + return fd (); + } + + // Open stream for reading. Return true if the file is not empty, + // false otherwise. Must not be called before wopen(). + // + bool + ropen () + { + // Otherwise compiler gets confused with basic_ios::fail(). + // + using build2::fail; + + assert (exists ()); + + try + { + open (path, in); + return peek () != ifdstream::traits_type::eof (); + } + catch (const io_error& e) + { + error << "unable to read " << path << ": " << e.what (); + throw failed (); + } + } + + // Return true if wopen() was called, false otherwise. + // + bool + exists () const {return !path.empty ();} + + ~stream_cache () override + { + close (); // Close the stream prior to the file deletion. + } + + public: + string name; + path_type path; + auto_rm cleanup; + }; + + // Check if the test command output matches the pattern (redirect value). + // + static void + check_output (const process_path& program, + stream_cache& sc, + const redirect& rd) + { + if (rd.type == redirect_type::none) + { + // Check that the cache file is empty. + // + if (sc.ropen ()) + { + sc.cleanup.cancel (); + + fail << program << " unexpectedly writes to " << sc.name << + info << sc.name << " is saved to " << sc.path; + } + } + else if (rd.type == redirect_type::here_string || + rd.type == redirect_type::here_document) + { + // Use diff utility to compare the output with the pattern. + // + path dp ("diff"); + process_path pp (run_search (dp, true)); + + cstrings args { + pp.recall_string (), + "--strip-trailing-cr", + "-u", + "-", + sc.path.string ().c_str (), + nullptr}; + + if (verb >= 2) + print_process (args); + + try + { + // Diff utility prints the differences to stdout. But for the user + // it is a part of the test failure diagnostics so let's redirect + // stdout to stderr. + // + process pr (pp, args.data (), -1, 2); + + try + { + ofdstream os (pr.out_fd); + + auto write_value = [&os, &rd] () + { + os << rd.value; + + // Here-document is always endline-terminated. + // + if (rd.type == redirect_type::here_string) + os << endl; + + os.close (); + }; + + write_value (); + + if (pr.wait ()) + return; + + // Output doesn't match the pattern string. Keep non-empty output + // and save the pattern for troubleshooting. + // + path p (sc.path + ".pattern"); + + try + { + os.open (p); + write_value (); + } + catch (const io_error& e) + { + fail << "unable to write " << p << ": " << e.what (); + } + + diag_record d (error); + d << program << " " << sc.name + << " doesn't match the pattern"; + + if (sc.ropen ()) + { + sc.cleanup.cancel (); + d << info << sc.name << " is saved to " << sc.path; + } + else + d << info << sc.name << " is empty"; + + // Pattern is never empty (contains at least newline). + // + d << info << sc.name << " pattern is saved to " << p; + + // Fall through. + // + } + catch (const io_error& e) + { + // Child exit status doesn't matter. Assume the child process + // issued diagnostics. Just wait for the process completion. + // + pr.wait (); // Check throw. + } + + // Fall through. + // + } + catch (const process_error& e) + { + error << "unable to execute " << args[0] << ": " << e.what (); + + if (e.child ()) + exit (1); + } + + throw failed (); + } + } + void concurrent_runner:: run (const test& t) { - // @@ TODO - // @@ When running multiple threads will need to synchronize printing // the diagnostics so it don't overlap for concurrent tests. // Alternatively we can not bother with that and expect a user to @@ -42,6 +271,182 @@ namespace build2 if (verb >= 3) print_test (t); + + // Pre-search the program path so it is reflected in the failure + // diagnostics. The user can see the original path running the test + // operation with the verbosity level > 2. + // + process_path pp (run_search (t.program, true)); + cstrings args {pp.recall_string ()}; + + for (const auto& a: t.arguments) + args.push_back (a.c_str ()); + + args.push_back (nullptr); + + // Normally while handling child process failures (IO errors, non-zero + // exit status) we suppress the diagnostics supposing that the child + // issues it's own one. While this is reasonable to expect from known + // production-quality programs here it can result in the absense of any + // diagnostics at all. Also the child stderr (and so diagnostics) can + // be redirected to /dev/null and not be available for the user. This + // why we will always issue the diagnostics despite the fact sometimes + // it can look redundant. + // + try + { + // For stdin 'none' redirect type we somehow need to make sure that + // the child process doesn't read from stdin. That is tricky to do in + // a portable way. Here we suppose that the program which + // (erroneously) tries to read some data from stdin being redirected + // to /dev/null fails not being able to do read expected data, and so + // the test doesn't pass through. + // + // @@ Obviously doesn't cover the case when the process reads + // whatever available. + // @@ Another approach could be not to redirect stdin and let the + // process to hang which can be interpreted as a test failure. + // @@ Both ways are quite ugly. Is there some better way to do this? + // + int in (t.in.type == redirect_type::null || + t.in.type == redirect_type::none + ? -2 + : -1); + + // Dealing with stdout and stderr redirect types other than 'null' + // using pipes is tricky in the general case. Going this path we + // would need to read both streams in a non-blocking manner which we + // can't (easily) do in a portable way. Using diff utility to get a + // nice-looking stream/pattern difference would complicate things + // further. + // + // So the approach is the following. Child standard stream are + // redirected to files. When the child exits and the exit status is + // validated we just sequentially compare each file content with the + // corresponding pattern. The positive side-effect of this approach + // is that the output of a faulty test command can be provided for + // troubleshooting. + // + stream_cache osc ("stdout"); + int out (t.out.type == redirect_type::null ? -2 : osc.wopen ()); + + stream_cache esc ("stderr"); + int err (t.err.type == redirect_type::null ? -2 : esc.wopen ()); + + if (verb >= 2) + print_process (args); + + process pr (pp, args.data (), in, out, err); + + try + { + osc.close (); + esc.close (); + + if (t.in.type == redirect_type::here_string || + t.in.type == redirect_type::here_document) + { + ofdstream os (pr.out_fd); + os << t.in.value; + + // Here-document is always endline-terminated. + // + if (t.in.type == redirect_type::here_string) + os << endl; + + os.close (); + } + + // Just wait. The program failure can mean the test success. + // + pr.wait (); + + // Check if the process terminated normally and obtain the status + // if that's the case. + // + bool abnorm; + process::status_type status; + + // @@ Shouldn't we incorporate means for checking for abnormal + // termination and getting the real exit status into + // libbutl::process? + // +#ifndef _WIN32 + abnorm = !WIFEXITED (pr.status); + status = abnorm ? 1 : WEXITSTATUS (pr.status); +#else + // @@ Is there a reliable way to detect if the process terminated + // abnormally on Windows? + // + abnorm = false; + status = pr.status; +#endif + bool valid_status (!abnorm && status >= 0 && status < 256); + + bool eq (t.exit.comparison == exit_comparison::eq); + + bool correct_status (valid_status && + (status == t.exit.status) == eq); + + // If there is no correct exit status by whatever reason then dump + // stderr (if cached), keep both stdout and stderr (those which + // are cached) for troubleshooting, and finally fail. + // + if (!correct_status) + { + if (esc.exists () && esc.ropen ()) + cerr << esc.rdbuf (); + + // Keep non-empty cache files and fail with a proper diagnostics. + // + diag_record d (fail); + + if (abnorm) + d << pp << " terminated abnormally"; + else if (!valid_status) + d << pp << " exit status " << status << " is invalid" << + info << "must be an unsigned integer < 256"; + else if (!correct_status) + d << pp << " exit status " << status + << (eq ? " != " : " == ") << (int)t.exit.status; + else + assert (false); + + auto keep_stream = [&d] (stream_cache& sc) + { + if (sc.exists () && sc.ropen ()) + { + sc.cleanup.cancel (); + d << info << sc.name << " is saved to " << sc.path; + } + }; + + keep_stream (osc); + keep_stream (esc); + } + + check_output (pp, osc, t.out); + check_output (pp, esc, t.err); + } + catch (const io_error& e) + { + // Child exit status doesn't matter. Just wait for the process + // completion. + // + pr.wait (); // Check throw. + + fail << "IO operation failed for " << pp << ": " << e.what (); + } + } + catch (const process_error& e) + { + error << "unable to execute " << pp << ": " << e.what (); + + if (e.child ()) + exit (1); + + throw failed (); + } } } } diff --git a/build2/test/script/script b/build2/test/script/script index 04b964f..236b025 100644 --- a/build2/test/script/script +++ b/build2/test/script/script @@ -97,6 +97,13 @@ namespace build2 // exit_comparison comparison = exit_comparison::eq; uint8_t status = 0; + + // Required by VC which fails to initialize as aggregate a class with + // default member initializers. + // + command_exit () = default; + command_exit (exit_comparison c, uint8_t s) + : comparison (c), status (s) {} }; struct test: command diff --git a/tests/buildfile b/tests/buildfile index 6367b5c..b6799d5 100644 --- a/tests/buildfile +++ b/tests/buildfile @@ -2,6 +2,6 @@ # copyright : Copyright (c) 2014-2016 Code Synthesis Ltd # license : MIT; see accompanying LICENSE file -d = +d = test/ ./: $d include $d diff --git a/tests/test/buildfile b/tests/test/buildfile new file mode 100644 index 0000000..0a98d5e --- /dev/null +++ b/tests/test/buildfile @@ -0,0 +1,7 @@ +# file : tests/test/buildfile +# copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +# license : MIT; see accompanying LICENSE file + +d = script/ +./: $d +include $d diff --git a/tests/test/script/buildfile b/tests/test/script/buildfile new file mode 100644 index 0000000..bf69e27 --- /dev/null +++ b/tests/test/script/buildfile @@ -0,0 +1,7 @@ +# file : tests/test/script/buildfile +# copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +# license : MIT; see accompanying LICENSE file + +exe{driver}: cxx{driver} test{testscript} + +include ../../../../build2/ diff --git a/tests/test/script/driver.cxx b/tests/test/script/driver.cxx new file mode 100644 index 0000000..b81172e --- /dev/null +++ b/tests/test/script/driver.cxx @@ -0,0 +1,76 @@ +// file : tests/test/script/driver.cxx -*- C++ -*- +// copyright : Copyright (c) 2014-2016 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#include // numeric_limits +#include +#include +#include // endl, *bit +#include +#include + +using namespace std; + +int +main (int argc, char* argv[]) +{ + // Usage: driver [-i ] [-s ] (-o )* (-e )* + // + int status (256); + int ifd (3); + + cout.exceptions (ostream::failbit | ostream::badbit); + cerr.exceptions (ostream::failbit | ostream::badbit); + + for (int i (1); i < argc; ++i) + { + string o (argv[i++]); + assert (i < argc); + + string v (argv[i]); + + auto toi = [] (const string& s) -> int + { + try + { + return stoi (s); + } + catch (const exception&) + { + assert (false); + } + }; + + if (o == "-i") + { + assert (ifd == 3); // Make sure is not set yet. + + ifd = toi (v); + assert (ifd >= 0 && ifd < 3); + + if (ifd == 0) + cin.ignore (numeric_limits::max ()); + else + (ifd == 1 ? cout : cerr) << cin.rdbuf (); + } + else if (o == "-o") + { + cout << v << endl; + } + else if (o == "-e") + { + cerr << v << endl; + } + else if (o == "-s") + { + assert (status == 256); // Make sure is not set yet. + + status = toi (v); + assert (status >= 0 && status < 256); + } + else + assert (false); + } + + return status == 256 ? 0 : status; +} diff --git a/tests/test/script/testscript b/tests/test/script/testscript new file mode 100644 index 0000000..b1cf0a5 --- /dev/null +++ b/tests/test/script/testscript @@ -0,0 +1,42 @@ +$* +$* -i 0 foo +$* -o foo >! +$* -e foo 2>! + +$* -o foo -o bar >>EOO +foo +bar +EOO + +$* -i 1 <>EOO +foo +bar +EOI +foo +bar +EOO + +$* -i 2 <>EOE +foo +bar +EOI +foo +bar +EOE + +$* -i 2 -s 1 <>EOE != 0 +foo +bar +EOI +foo +bar +EOE + +$* -i 2 -o baz -s 10 <baz 2>>EOE == 10 +foo +bar +EOI +foo +bar +EOE diff --git a/unit-tests/test/script/lexer/buildfile b/unit-tests/test/script/lexer/buildfile index 0d32710..a9f6be9 100644 --- a/unit-tests/test/script/lexer/buildfile +++ b/unit-tests/test/script/lexer/buildfile @@ -7,6 +7,6 @@ import libs = libbutl%lib{butl} src = token lexer diagnostics utility variable name test/script/{token lexer} -exe{driver}: cxx{driver} ../../../../build2/cxx{$src} $libs test{script-line} +exe{driver}: cxx{driver} ../../../../build2/cxx{$src} $libs test{variable.test} include ../../../../build2/ -- cgit v1.1