diff options
| -rw-r--r-- | build2/test/script/runner | 4 | ||||
| -rw-r--r-- | build2/test/script/runner.cxx | 413 | ||||
| -rw-r--r-- | build2/test/script/script | 2 | ||||
| -rw-r--r-- | build2/utility.cxx | 3 |
4 files changed, 202 insertions, 220 deletions
diff --git a/build2/test/script/runner b/build2/test/script/runner index c4f87ee..0180108 100644 --- a/build2/test/script/runner +++ b/build2/test/script/runner @@ -47,13 +47,13 @@ namespace build2 { public: virtual void - enter (scope&, const location&) override {} + enter (scope&, const location&) override; virtual void run (scope&, const command&, size_t, const location&) override; virtual void - leave (scope&, const location&) override {} + leave (scope&, const location&) override; }; } } diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx index a4fb31e..510b61d 100644 --- a/build2/test/script/runner.cxx +++ b/build2/test/script/runner.cxx @@ -2,14 +2,10 @@ // copyright : Copyright (c) 2014-2016 Code Synthesis Ltd // license : MIT; see accompanying LICENSE file -#ifndef _WIN32 -# include <sys/wait.h> // WIFEXITED(), WEXITSTATUS() -#endif - +#include <set> #include <iostream> // cerr -#include <butl/filesystem> // auto_rm - +#include <build2/filesystem> #include <build2/test/script/runner> using namespace std; @@ -21,141 +17,105 @@ namespace build2 { namespace script { - // 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. + // @@ Eventually the cleanup facility should move into the scope object. // - // 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 // @@ Open for output? + class cleanup { public: - using path_type = butl::path; + void + add (path p) {files_.emplace (move (p));} - stream_cache (const char* n): name (n) {} + void + cancel (const path& p) {files_.erase (p);} - // Open stream for writing. Return the file descriptor. Must not be - // called multiple times. - // - int - wopen () + ~cleanup () { - // 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_type> (path); - } - catch (const io_error& e) - { - fail << "unable to write " << path << ": " << e.what (); - } - - return fd (); + for (const auto& p: files_) + try_rmfile (p, true); } - // 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; + private: + set<path> files_; + }; - assert (exists ()); + // Check if the file exists and is not empty. + // + static bool + non_empty (const path& p) + { + if (!exists (p)) + return false; - try - { - open (path, in); - return peek () != ifdstream::traits_type::eof (); - } - catch (const io_error& e) - { - error << "unable to read " << path << ": " << e.what (); - throw failed (); - } + try + { + ifdstream is (p); + return is.peek () != ifdstream::traits_type::eof (); } - - // Return true if wopen() was called, false otherwise. - // - bool - exists () const {return !path.empty ();} - - ~stream_cache () override + catch (const io_error& e) { - close (); // Close the stream prior to the file deletion. + error << "unable to read " << p << ": " << e.what (); + throw failed (); } + } - public: - string name; - path_type path; - auto_rm<path_type> cleanup; - }; - - // Check if the test command output matches the pattern (redirect value). - // - // @@ Expected result | expected output + // Check if the test command output matches the expected result (redirect + // value). // static void - check_output (const process_path& program, - stream_cache& sc, - const redirect& rd) + check_output (const process_path& pr, + const char* nm, + const path& op, + const redirect& rd, + const location& cl, + cleanup& cln) { if (rd.type == redirect_type::none) { - // Check that the cache file is empty. + // Check that there is no output produced. // - if (sc.ropen ()) + if (non_empty (op)) { - sc.cleanup.cancel (); + cln.cancel (op); - fail << program << " unexpectedly writes to " << sc.name << - info << sc.name << " is saved to " << sc.path; + fail (cl) << pr << " unexpectedly writes to " << nm << + info << nm << " is saved to " << op; } } 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 orp (op + ".orig"); + + try + { + ofdstream os (orp); + cln.add (orp); + + os << rd.value; + + // Here-document is always newline-terminated. + // + if (rd.type == redirect_type::here_string) + os << endl; + + os.close (); + } + catch (const io_error& e) + { + fail << "unable to write " << orp << ": " << e.what (); + } + + // Use diff utility to compare the output with the expected result. // path dp ("diff"); process_path pp (run_search (dp, true)); - //@@ Need to compare both as files. - cstrings args { pp.recall_string (), "--strip-trailing-cr", "-u", - "-", - sc.path.string ().c_str (), + orp.string ().c_str (), + op.string ().c_str (), nullptr}; if (verb >= 2) @@ -167,71 +127,45 @@ namespace build2 // it is a part of the test failure diagnostics so let's redirect // stdout to stderr. // - process pr (pp, args.data (), -1, 2); + process p (pp, args.data (), 0, 2); try { - ofdstream os (pr.out_fd); - - auto write_value = [&os, &rd] () - { - os << rd.value; - - // Here-document is always endline-terminated. - // - // @@ newline - // - if (rd.type == redirect_type::here_string) - os << endl; - - os.close (); - }; - - write_value (); - - if (pr.wait ()) + if (p.wait ()) return; - // Output doesn't match the pattern string. Keep non-empty output - // and save the pattern for troubleshooting. + // Output doesn't match the expected result. Keep non-empty + // output and the expected result for troubleshooting. // - path p (sc.path + ".pattern"); // @@ .orig + cln.cancel (orp); - try - { - os.open (p); - write_value (); - } - catch (const io_error& e) - { - fail << "unable to write " << p << ": " << e.what (); - } + diag_record d (error (cl)); + d << pr << " " << nm << " doesn't match the expected output"; - diag_record d (error); - d << program << " " << sc.name - << " doesn't match the pattern"; - - if (sc.ropen ()) + if (non_empty (op)) { - sc.cleanup.cancel (); - d << info << sc.name << " is saved to " << sc.path; + cln.cancel (op); + d << info << nm << " is saved to " << op; } else - d << info << sc.name << " is empty"; + d << info << nm << " is empty"; - // Pattern is never empty (contains at least newline). + // Expected output is never empty (contains at least newline). // - d << info << sc.name << " pattern is saved to " << p; + d << info << "expected " << nm << " is saved to " << orp; // Fall through. // } - catch (const io_error& e) + catch (const io_error&) { // Child exit status doesn't matter. Assume the child process // issued diagnostics. Just wait for the process completion. // - pr.wait (); // Check throw. + p.wait (); // Check throw. + + error (cl) << "failed to compare " << nm + << " with the expected output"; } // Fall through. @@ -239,7 +173,7 @@ namespace build2 } catch (const process_error& e) { - error << "unable to execute " << args[0] << ": " << e.what (); + error (cl) << "unable to execute " << pp << ": " << e.what (); if (e.child ()) exit (1); @@ -250,7 +184,26 @@ namespace build2 } void concurrent_runner:: - run (scope&, const command& c, size_t ci, const location& cl) + enter (scope& sp, const location&) + { + if (!exists (sp.wd_path)) + mkdir (sp.wd_path, 2); + else if (!empty (sp.wd_path)) + // @@ Shouldn't we have --wipe or smth? + // + fail << "directory " << sp.wd_path << " is not empty" << + info << "clean it up and rerun"; + } + + void concurrent_runner:: + leave (scope& sp, const location&) + { + if (exists (sp.wd_path) && empty (sp.wd_path)) + rmdir (sp.wd_path, 2); + } + + void concurrent_runner:: + run (scope& sp, const command& c, size_t ci, const location& cl) { if (verb >= 3) text << c; @@ -267,15 +220,6 @@ namespace build2 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 @@ -300,31 +244,69 @@ namespace build2 // 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. + // nice-looking actual/expected output 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 + // expected output. 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 (c.out.type == redirect_type::null ? -2 : osc.wopen ()); + cleanup cln; + + auto opath = [sp, ci] (const char* nm) -> path + { + path r (sp.wd_path / path (nm)); - stream_cache esc ("stderr"); - int err (c.err.type == redirect_type::null ? -2 : esc.wopen ()); + if (ci > 0) + r += "-" + to_string (ci); + + return r; + }; + + auto open = [&cln] (ofdstream& os, const path& p) -> int + { + try + { + os.open (p); + cln.add (p); + } + catch (const io_error& e) + { + fail << "unable to write " << p << ": " << e.what (); + } + + return os.fd (); + }; + + ofdstream so; + path stdout (opath ("stdout")); + + int out (c.out.type == redirect_type::null + ? -2 + : open (so, stdout)); + + ofdstream se; + path stderr (opath ("stderr")); + + int err (c.err.type == redirect_type::null + ? -2 + : open (se, stderr)); if (verb >= 2) print_process (args); - process pr (pp, args.data (), in, out, err); + process pr (sp.wd_path.string ().c_str (), + pp, + args.data (), + in, out, err); try { - osc.close (); - esc.close (); + so.close (); + se.close (); if (c.in.type == redirect_type::here_string || c.in.type == redirect_type::here_document) @@ -332,7 +314,7 @@ namespace build2 ofdstream os (pr.out_fd); os << c.in.value; - // Here-document is always endline-terminated. + // Here-document is always newline-terminated. // if (c.in.type == redirect_type::here_string) os << endl; @@ -344,74 +326,69 @@ namespace build2 // 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? - // - // Yes, sounds good. - // -#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? + // 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, print the proper diagnostics + // and finally fail. // - abnorm = false; - status = pr.status; -#endif - bool valid_status (!abnorm && status >= 0 && status < 256); - + optional<process::status_type> status (move (pr.status)); + bool valid_status (status && *status >= 0 && *status < 256); bool eq (c.exit.comparison == exit_comparison::eq); bool correct_status (valid_status && - (status == c.exit.status) == eq); + (*status == c.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 (); + // Dump cached stderr. + // + if (exists (stderr)) + { + try + { + ifdstream is (stderr); + if (is.peek () != ifdstream::traits_type::eof ()) + cerr << is.rdbuf (); + } + catch (const io_error& e) + { + fail << "unable to read " << stderr << ": " << e.what (); + } + } // Keep non-empty cache files and fail with a proper diagnostics. // - diag_record d (fail); + diag_record d (fail (cl)); - if (abnorm) + if (!status) d << pp << " terminated abnormally"; else if (!valid_status) - d << pp << " exit status " << status << " is invalid" << + 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)c.exit.status; //@@ + d << pp << " exit status " << *status + << (eq ? " != " : " == ") + << static_cast<uint16_t> (c.exit.status); else assert (false); - auto keep_stream = [&d] (stream_cache& sc) + auto keep_output = [&d, &cln] (const char* name, const path& p) { - if (sc.exists () && sc.ropen ()) + if (non_empty (p)) { - sc.cleanup.cancel (); - d << info << sc.name << " is saved to " << sc.path; + cln.cancel (p); + d << info << name << " is saved to " << p; } }; - keep_stream (osc); - keep_stream (esc); + keep_output ("stdout", stdout); + keep_output ("stderr", stderr); } - check_output (pp, osc, c.out); - check_output (pp, esc, c.err); + // Check if the standard outputs match expectations. + // + check_output (pp, "stdout", stdout, c.out, cl, cln); + check_output (pp, "stderr", stderr, c.err, cl, cln); } catch (const io_error& e) { @@ -420,12 +397,12 @@ namespace build2 // pr.wait (); // Check throw. - fail << "IO operation failed for " << pp << ": " << e.what (); + fail (cl) << "IO operation failed for " << pp << ": " << e.what (); } } catch (const process_error& e) { - error << "unable to execute " << pp << ": " << e.what (); + error (cl) << "unable to execute " << pp << ": " << e.what (); if (e.child ()) exit (1); diff --git a/build2/test/script/script b/build2/test/script/script index 3095ccf..ff9fa46 100644 --- a/build2/test/script/script +++ b/build2/test/script/script @@ -22,6 +22,8 @@ namespace build2 { namespace script { + class parser; // Required by VC for 'friend class parser' declaration. + // Pre-parse representation. // enum class line_type {variable, test}; diff --git a/build2/utility.cxx b/build2/utility.cxx index 0e97020..ca6b239 100644 --- a/build2/utility.cxx +++ b/build2/utility.cxx @@ -36,6 +36,9 @@ namespace build2 os << "<empty>"; else { + // @@ Is there a reason not to print as a relative path as it is done + // for path (see above)? + // os << p.recall_string (); if (!p.effect.empty ()) |