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 + 2 files changed, 414 insertions(+), 2 deletions(-) (limited to 'build2/test') 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 -- cgit v1.1