From 5daf46f700217521e8ba90c4be0e0369105544df Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 26 Oct 2016 12:32:46 +0300 Subject: Add support of file redirects to testscript runner --- build2/test/script/parser.cxx | 4 +- build2/test/script/runner.cxx | 188 ++++++++++++++++++------- tests/test/script/runner/driver.cxx | 5 +- tests/test/script/runner/testscript | 9 +- unit-tests/test/script/lexer/buildfile | 3 +- unit-tests/test/script/lexer/command-line.test | 107 ++++++++++++++ unit-tests/test/script/lexer/script-line.test | 68 +++++++++ unit-tests/test/script/parser/buildfile | 3 +- unit-tests/test/script/parser/redirect.test | 5 + 9 files changed, 331 insertions(+), 61 deletions(-) create mode 100644 unit-tests/test/script/lexer/command-line.test create mode 100644 unit-tests/test/script/parser/redirect.test diff --git a/build2/test/script/parser.cxx b/build2/test/script/parser.cxx index 3043f05..ae8df12 100644 --- a/build2/test/script/parser.cxx +++ b/build2/test/script/parser.cxx @@ -616,7 +616,7 @@ namespace build2 } catch (const invalid_path& e) { - fail (l) << "invalid " << n << "redirect file path '" << e.path + fail (l) << "invalid " << n << " redirect file path '" << e.path << "'"; } @@ -625,7 +625,7 @@ namespace build2 switch (p) { - case pending::none: c.arguments.push_back (move (w)); break; + case pending::none: c.arguments.push_back (move (w)); break; case pending::program: { try diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx index f065389..eb6531d 100644 --- a/build2/test/script/runner.cxx +++ b/build2/test/script/runner.cxx @@ -4,6 +4,7 @@ #include +#include #include // cerr #include @@ -16,12 +17,13 @@ namespace build2 { namespace script { - // Check if the file exists and is not empty. + // Check if a path is not empty, the referenced file exists and is not + // empty. // static bool - non_empty (const path& p) + non_empty (const path& p, const location& cl) { - if (!exists (p)) + if (p.empty () || !exists (p)) return false; try @@ -31,7 +33,11 @@ namespace build2 } catch (const io_error& e) { - error << "unable to read " << p << ": " << e.what (); + // While there can be no fault of the test command being currently + // executed let's add the location anyway to ease the + // troubleshooting. And let's stick to that principle down the road. + // + error (cl) << "unable to read " << p << ": " << e.what (); throw failed (); } } @@ -51,7 +57,7 @@ namespace build2 { // Check that there is no output produced. // - if (non_empty (op)) + if (non_empty (op, cl)) fail (cl) << pr << " unexpectedly writes to " << nm << info << nm << " is saved to " << op; } @@ -73,7 +79,7 @@ namespace build2 } catch (const io_error& e) { - fail << "unable to write " << orp << ": " << e.what (); + fail (cl) << "unable to write " << orp << ": " << e.what (); } // Use diff utility to compare the output with the expected result. @@ -110,9 +116,10 @@ namespace build2 diag_record d (error (cl)); d << pr << " " << nm << " doesn't match the expected output"; - auto output_info = [&d, &nm] (const path& p, const char* prefix) + auto output_info = + [&d, &nm, &cl] (const path& p, const char* prefix) { - if (non_empty (p)) + if (non_empty (p, cl)) d << info << prefix << nm << " is saved to " << p; else d << info << prefix << nm << " is empty"; @@ -151,14 +158,18 @@ namespace build2 } void concurrent_runner:: - enter (scope& sp, const location&) + enter (scope& sp, const location& cl) { if (!exists (sp.wd_path)) + // @@ Shouldn't we add an optional location parameter to mkdir() and + // alike utility functions so the failure message can contain + // location info? + // 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" << + fail (cl) << "directory " << sp.wd_path << " is not empty" << info << "clean it up and rerun"; sp.cleanups.emplace_back (sp.wd_path); @@ -167,22 +178,34 @@ namespace build2 void concurrent_runner:: leave (scope& sp, const location& cl) { - for (const auto& p: reverse_iterate (sp.cleanups)) + // Remove files and directories in the order opposite to the order of + // cleanup registration. Handle paths multiple registration (which is a + // valid case). + // + // Note that we operate with normalized paths here. + // + set rp; + for (auto& p: reverse_iterate (sp.cleanups)) { - if (p.to_directory ()) + auto i (rp.emplace (move (p))); + if (i.second) // Remove the path if seen for the first time. { - dir_path d (path_cast (p)); - rmdir_status r (rmdir (d, 2)); - - if (r != rmdir_status::success) - fail (cl) << "registered for cleanup directory " << d - << (r == rmdir_status::not_empty - ? " is not empty" - : " does not exist"); + const path& p (*i.first); + if (p.to_directory ()) + { + dir_path d (path_cast (p)); + rmdir_status r (rmdir (d, 2)); + + if (r != rmdir_status::success) + fail (cl) << "registered for cleanup directory " << d + << (r == rmdir_status::not_empty + ? " is not empty" + : " does not exist"); + } + else if (rmfile (p, 2) == rmfile_status::not_exist) + fail (cl) << "registered for cleanup file " << p + << " does not exist"; } - else if (rmfile (p, 2) == rmfile_status::not_exist) - fail (cl) << "registered for cleanup file " << p - << " does not exist"; } } @@ -219,7 +242,28 @@ namespace build2 // process to hang which can be interpreted as a test failure. // @@ Both ways are quite ugly. Is there some better way to do this? // + + // Normalize a path. Also make relative path absolute using the + // scope's working directory unless it is already absolute. + // + auto normalize = [&sp, &cl] (path p) -> path + { + path r (p.absolute () ? move (p) : sp.wd_path / move (p)); + + try + { + r.normalize (); + } + catch (const invalid_path& e) + { + fail (cl) << "invalid file path " << e.path; + } + + return r; + }; + int in; + ifdstream si; switch (c.in.type) { @@ -230,6 +274,23 @@ namespace build2 case redirect_type::null: case redirect_type::none: in = -2; break; + + case redirect_type::file: + { + path p (normalize (c.in.file.path)); + + try + { + si.open (p); + } + catch (const io_error& e) + { + fail (cl) << "unable to read " << p << ": " << e.what (); + } + + in = si.fd (); + break; + } } // Dealing with stdout and stderr redirect types other than 'null' @@ -246,48 +307,66 @@ namespace build2 // the output of a faulty test command can be provided for // troubleshooting. // - auto opath = [sp, ci] (const char* nm) -> path + + // Open a file for command output redirect if requested explicitly + // (file redirect) or for the purpose of the output validation (none, + // here_string, here_document), register the file for cleanup, return + // the file descriptor. Return the default and -2 file descriptors + // for pass and null redirects respectively not opening a file. + // + auto open = [&sp, &ci, &cl, &normalize] (const redirect& r, + int fd, + path& p, + ofdstream& os) -> int { - path r (sp.wd_path / path (nm)); + assert (fd == 1 || fd == 2); - if (ci > 0) - r += "-" + to_string (ci + 1); // Start from first line. + if (r.type == redirect_type::pass || r.type == redirect_type::null) + return r.type == redirect_type::pass ? fd : -2; - return r; - }; + ofdstream::openmode m (ofdstream::out); + if (r.type == redirect_type::file) + { + p = normalize (r.file.path); + if (r.file.append) + m |= ofdstream::app; + } + else + { + path op (fd == 1 ? "stdout" : "stderr"); + + // 0 if a single-command test, otherwise is the command number + // (start from one) in the test. + // + if (ci > 0) + op += "-" + to_string (ci); + + p = normalize (move (op)); + } - auto open = [&sp] (ofdstream& os, const path& p) -> int - { try { - os.open (p); - sp.cleanups.emplace_back (p); + os.open (p, m); } catch (const io_error& e) { - fail << "unable to write " << p << ": " << e.what (); + fail (cl) << "unable to write " << p << ": " << e.what (); } + // It is a valid case if the file path is repeatedly registered for + // cleanup. It is handled during cleanup procedure. + // + sp.cleanups.emplace_back (p); return os.fd (); }; + path stdout; ofdstream so; - path stdout (opath ("stdout")); - - int out (c.out.type == redirect_type::pass - ? 1 - : c.out.type == redirect_type::null - ? -2 - : open (so, stdout)); + int out (open (c.out, 1, stdout, so)); + path stderr; ofdstream se; - path stderr (opath ("stderr")); - - int err (c.err.type == redirect_type::pass - ? 2 - : c.err.type == redirect_type::null - ? -2 - : open (se, stderr)); + int err (open (c.err, 2, stderr, se)); if (verb >= 2) print_process (args); @@ -299,14 +378,14 @@ namespace build2 try { + si.close (); so.close (); se.close (); - const redirect& r (c.in); - if (r.type == redirect_type::here_string || - r.type == redirect_type::here_document) + if (in == -1) // here_string, here_document redirects. { ofdstream os (pr.out_fd); + const redirect& r (c.in); os << (r.type == redirect_type::here_string ? r.str : r.doc.doc); os.close (); } @@ -339,7 +418,8 @@ namespace build2 } catch (const io_error& e) { - fail << "unable to read " << stderr << ": " << e.what (); + fail (cl) << "unable to read " << stderr << ": " + << e.what (); } } @@ -359,10 +439,10 @@ namespace build2 else assert (false); - if (non_empty (stdout)) + if (non_empty (stdout, cl)) d << info << "stdout is saved to " << stdout; - if (non_empty (stderr)) + if (non_empty (stderr, cl)) d << info << "stderr is saved to " << stderr; } diff --git a/tests/test/script/runner/driver.cxx b/tests/test/script/runner/driver.cxx index 9eb36a0..fdada73 100644 --- a/tests/test/script/runner/driver.cxx +++ b/tests/test/script/runner/driver.cxx @@ -34,7 +34,10 @@ main (int argc, char* argv[]) { try { - return stoi (s); + size_t n; + int r (stoi (s, &n)); + assert (n == s.size ()); + return r; } catch (const exception&) { diff --git a/tests/test/script/runner/testscript b/tests/test/script/runner/testscript index 292e8ec..c95ed07 100644 --- a/tests/test/script/runner/testscript +++ b/tests/test/script/runner/testscript @@ -2,8 +2,6 @@ # copyright : Copyright (c) 2014-2016 Code Synthesis Ltd # license : MIT; see accompanying LICENSE file -#$* -o foo <<>>&bbb 2>>>ccc - $* # status-def $* == 0 # status-eq-0 $* -s 1 != 0 # status-ne-0 @@ -110,3 +108,10 @@ $* -i 1 <<:EOI >>:EOO # no-newline-nl-cont-doc EOI EOO + +$* -o foo >>>out; # file-redirect +$* -e bar 2>>>&out; +$* -i 1 <<>EOO +foo +bar +EOO diff --git a/unit-tests/test/script/lexer/buildfile b/unit-tests/test/script/lexer/buildfile index fab61b6..30f6b9f 100644 --- a/unit-tests/test/script/lexer/buildfile +++ b/unit-tests/test/script/lexer/buildfile @@ -8,6 +8,7 @@ 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 first-token second-token variable-line variable comment} +test{script-line command-line first-token second-token variable-line variable \ + comment} include ../../../../build2/ diff --git a/unit-tests/test/script/lexer/command-line.test b/unit-tests/test/script/lexer/command-line.test new file mode 100644 index 0000000..765b59a --- /dev/null +++ b/unit-tests/test/script/lexer/command-line.test @@ -0,0 +1,107 @@ +test.arguments += command-line + +$* <:"0<+" >>EOO # in-pass-redirect +'0' +<+ +EOO + +$* <:"0 <+" >>EOO # arg-in-pass-redirect +'0 ' +<+ +EOO + +$* <:"1>+" >>EOO # out-pass-redirect +'1' +>+ +EOO + +$* <:"1 >+" >>EOO # arg-out-pass-redirect +'1 ' +>+ +EOO + +$* <:"0<-" >>EOO # in-null-redirect +'0' +<- +EOO + +$* <:"0 <-" >>EOO # arg-in-null-redirect +'0 ' +<- +EOO + +$* <:"1>-" >>EOO # out-null-redirect +'1' +>- +EOO + +$* <:"1 >-" >>EOO # arg-out-null-redirect +'1 ' +>- +EOO + +$* <:"0>EOO # in-str-redirect +'0' +< +'a b' +EOO + +$* <:"1>a b" >>EOO # out-str-redirect +'1' +> +'a b' +EOO + +$* <:"0<:a b" >>EOO # in-str-nn-redirect +'0' +<: +'a b' +EOO + +$* <:"1>:a b" >>EOO # out-str-nn-redirect +'1' +>: +'a b' +EOO + +$* <:"0<>EOO # in-doc-redirect +'0' +<< +'E O I' +EOO + +$* <:"1>>E O O" >>EOO # out-doc-redirect +'1' +>> +'E O O' +EOO + +$* <:"0<<:E O I" >>EOO # in-doc-nn-redirect +'0' +<<: +'E O I' +EOO + +$* <:"1>>:E O O" >>EOO # out-doc-nn-redirect +'1' +>>: +'E O O' +EOO + +$* <:"0<<>EOO # in-file-redirect +'0' +<<< +'a b' +EOO + +$* <:"1>>>a b" >>EOO # out-file-redirect +'1' +>>> +'a b' +EOO + +$* <:"1>>>&a b" >>EOO # out-file-app-redirect +'1' +>>>& +'a b' +EOO diff --git a/unit-tests/test/script/lexer/script-line.test b/unit-tests/test/script/lexer/script-line.test index b4fe3ef..a217591 100644 --- a/unit-tests/test/script/lexer/script-line.test +++ b/unit-tests/test/script/lexer/script-line.test @@ -16,3 +16,71 @@ $* <";" >>EOO # semi-only ; EOO + +$* <"cmd <+ 1>+" >>EOO # pass-redirect +'cmd' +<+ +'1' +>+ + +EOO + +$* <"cmd <- 1>-" >>EOO # null-redirect +'cmd' +<- +'1' +>- + +EOO + +$* <"cmd b" >>EOO # str-redirect +'cmd' +< +'a' +'1' +> +'b' + +EOO + +$* <"cmd <:a 1>:b" >>EOO # str-nn-redirect +'cmd' +<: +'a' +'1' +>: +'b' + +EOO + +$* <"cmd <>EOO" >>EOO # doc-redirect +'cmd' +<< +'EOI' +'1' +>> +'EOO' + +EOO + +$* <"cmd <<:EOI 1>>:EOO" >>EOO # doc-nn-redirect +'cmd' +<<: +'EOI' +'1' +>>: +'EOO' + +EOO + +$* <"cmd <<>>out 2>>>&err" >>EOO # file-redirect +'cmd' +<<< +'in' +>>> +'out' +'2' +>>>& +'err' + +EOO diff --git a/unit-tests/test/script/parser/buildfile b/unit-tests/test/script/parser/buildfile index 18acd20..b053971 100644 --- a/unit-tests/test/script/parser/buildfile +++ b/unit-tests/test/script/parser/buildfile @@ -11,6 +11,7 @@ filesystem config/{utility init operation} dump types-parsers \ test/{target script/{token lexer parser script}} exe{driver}: cxx{driver} ../../../../build2/cxx{$src} $libs \ -test{pre-parse expansion here-document command-re-parse scope setup-teardown} +test{pre-parse expansion redirect here-document command-re-parse scope \ + setup-teardown} include ../../../../build2/ diff --git a/unit-tests/test/script/parser/redirect.test b/unit-tests/test/script/parser/redirect.test new file mode 100644 index 0000000..83b40cd --- /dev/null +++ b/unit-tests/test/script/parser/redirect.test @@ -0,0 +1,5 @@ +$* <>EOO # file +cmd 0<<>>b 2>>>&c +EOI +cmd <<>>b 2>>>&c +EOO -- cgit v1.1