From 52a29dd64479e9313848941078a2619b47f2c61b Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 8 Mar 2017 00:39:30 +0300 Subject: Add support for config.test.output variable --- build2/test/common | 4 +- build2/test/rule.cxx | 15 +- build2/test/script/parser | 2 +- build2/test/script/parser.cxx | 19 ++- build2/test/script/runner | 10 +- build2/test/script/runner.cxx | 215 ++++++++++++++------------ build2/test/script/script | 7 + build2/test/script/script.cxx | 17 ++ tests/test/script/runner/buildfile | 2 +- tests/test/script/runner/output.test | 87 +++++++++++ unit-tests/test/script/parser/command-if.test | 4 +- unit-tests/test/script/parser/driver.cxx | 4 +- unit-tests/test/script/parser/scope-if.test | 4 +- unit-tests/test/script/parser/scope.test | 2 +- 14 files changed, 269 insertions(+), 123 deletions(-) create mode 100644 tests/test/script/runner/output.test diff --git a/build2/test/common b/build2/test/common index 8e4235b..1a3117a 100644 --- a/build2/test/common +++ b/build2/test/common @@ -21,8 +21,8 @@ namespace build2 { // The config.test.output values. // - output_before before; - output_after after; + output_before before = output_before::warn; + output_after after = output_after::clean; // The config.test query interface. // diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx index 33ffb90..6ef2587 100644 --- a/build2/test/rule.cxx +++ b/build2/test/rule.cxx @@ -422,9 +422,16 @@ namespace build2 if (exists (wd)) { - warn << "working directory " << wd << " exists " - << (empty (wd) ? "" : "and is not empty ") << "at the beginning " - << "of the test"; + if (before != output_before::clean) + { + bool fail (before == output_before::fail); + + (fail ? error : warn) << "working directory " << wd << " exists " + << (empty (wd) ? "" : "and is not empty ") + << "at the beginning of the test"; + if (fail) + throw failed (); + } // Remove the directory itself not to confuse the runner which tries // to detect when tests stomp on each others feet. @@ -533,7 +540,7 @@ namespace build2 // Cleanup. // - if (!one && !mk) + if (!one && !mk && after == output_after::clean) { if (!empty (wd)) fail << "working directory " << wd << " is not empty at the " diff --git a/build2/test/script/parser b/build2/test/script/parser index cb536e6..e018548 100644 --- a/build2/test/script/parser +++ b/build2/test/script/parser @@ -184,7 +184,7 @@ namespace build2 exec_scope_body (); void - exec_lines (lines::iterator, lines::iterator, size_t&, bool); + exec_lines (lines::iterator, lines::iterator, size_t&, command_type); // Customization hooks. // diff --git a/build2/test/script/parser.cxx b/build2/test/script/parser.cxx index 9d17304..5a33007 100644 --- a/build2/test/script/parser.cxx +++ b/build2/test/script/parser.cxx @@ -2866,11 +2866,13 @@ namespace build2 if (test* t = dynamic_cast (scope_)) { - exec_lines (t->tests_.begin (), t->tests_.end (), li, true); + exec_lines ( + t->tests_.begin (), t->tests_.end (), li, command_type::test); } else if (group* g = dynamic_cast (scope_)) { - exec_lines (g->setup_.begin (), g->setup_.end (), li, false); + exec_lines ( + g->setup_.begin (), g->setup_.end (), li, command_type::setup); atomic_count task_count (0); wait_guard wg (task_count); @@ -3017,7 +3019,8 @@ namespace build2 } } - exec_lines (g->tdown_.begin (), g->tdown_.end (), li, false); + exec_lines ( + g->tdown_.begin (), g->tdown_.end (), li, command_type::teardown); } else assert (false); @@ -3028,7 +3031,9 @@ namespace build2 } void parser:: - exec_lines (lines::iterator i, lines::iterator e, size_t& li, bool test) + exec_lines (lines::iterator i, lines::iterator e, + size_t& li, + command_type ct) { token t; type tt; @@ -3096,7 +3101,7 @@ namespace build2 // We use the 0 index to signal that this is the only command. // Note that we only do this for test commands. // - if (test && li == 0) + if (ct == command_type::test && li == 0) { lines::iterator j (i); for (++j; j != e && j->type == line_type::var; ++j) ; @@ -3108,7 +3113,7 @@ namespace build2 ++li; command_expr ce (parse_command_line (t, tt)); - runner_->run (*scope_, ce, li, ll); + runner_->run (*scope_, ce, ct, li, ll); replay_stop (); break; @@ -3207,7 +3212,7 @@ namespace build2 if (take) { lines::iterator j (next (i, false, false)); // Next if-else. - exec_lines (i + 1, j, li, test); + exec_lines (i + 1, j, li, ct); i = j->type == line_type::cmd_end ? j : next (j, true, true); } else diff --git a/build2/test/script/runner b/build2/test/script/runner index 56ea834..566b9de 100644 --- a/build2/test/script/runner +++ b/build2/test/script/runner @@ -42,7 +42,10 @@ namespace build2 // testscript. It can be used in diagnostics. // virtual void - run (scope&, const command_expr&, size_t index, const location&) = 0; + run (scope&, + const command_expr&, command_type, + size_t index, + const location&) = 0; virtual bool run_if (scope&, const command_expr&, size_t, const location&) = 0; @@ -66,7 +69,10 @@ namespace build2 enter (scope&, const location&) override; virtual void - run (scope&, const command_expr&, size_t, const location&) override; + run (scope&, + const command_expr&, command_type, + size_t, + const location&) override; virtual bool run_if (scope&, const command_expr&, size_t, const location&) override; diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx index 5991546..bcaca44 100644 --- a/build2/test/script/runner.cxx +++ b/build2/test/script/runner.cxx @@ -667,136 +667,144 @@ namespace build2 void default_runner:: leave (scope& sp, const location& ll) { - // Remove files and directories in the order opposite to the order of - // cleanup registration. + // Perform registered cleanups if requested. // - // Note that we operate with normalized paths here. - // - for (const auto& c: reverse_iterate (sp.cleanups)) + if (common_.after == output_after::clean) { - cleanup_type t (c.type); - - // Skip whenever the path exists or not. + // Remove files and directories in the order opposite to the order of + // cleanup registration. // - if (t == cleanup_type::never) - continue; - - const path& p (c.path); - const path& l (p.leaf ()); - const string& ls (l.string ()); - - // Remove the directory recursively if not current. Fail otherwise. - // Recursive removal of non-existing directory is not an error for - // 'maybe' cleanup type. + // Note that we operate with normalized paths here. // - if (ls == "***") + for (const auto& c: reverse_iterate (sp.cleanups)) { - // Cast to uint16_t to avoid ambiguity with libbutl::rmdir_r(). - // - rmdir_status r ( - rmdir_r (p.directory (), true, static_cast (2))); + cleanup_type t (c.type); - if (r == rmdir_status::success || - (r == rmdir_status::not_exist && t == cleanup_type::maybe)) + // Skip whenever the path exists or not. + // + if (t == cleanup_type::never) continue; - // The directory is unlikely to be current but let's keep for - // completeness. + const path& p (c.path); + const path& l (p.leaf ()); + const string& ls (l.string ()); + + // Remove the directory recursively if not current. Fail otherwise. + // Recursive removal of non-existing directory is not an error for + // 'maybe' cleanup type. // - fail (ll) << "registered for cleanup wildcard " << p - << (r == rmdir_status::not_empty - ? " matches the current directory" - : " doesn't match a directory"); - } + if (ls == "***") + { + // Cast to uint16_t to avoid ambiguity with libbutl::rmdir_r(). + // + rmdir_status r ( + rmdir_r (p.directory (), true, static_cast (2))); - // Remove files or directories using wildcard. Removal of sub-entries - // of non-existing directory is not an error for 'maybe' cleanup - // type. - // - // Note that only the leaf part of the cleanup wildcard is a pattern - // in terms of libbutl::path_search(). - // - if (ls == "*" || ls == "**") - { - dir_path d (p.directory ()); + if (r == rmdir_status::success || + (r == rmdir_status::not_exist && t == cleanup_type::maybe)) + continue; - if (t == cleanup_type::always && !dir_exists (d)) + // The directory is unlikely to be current but let's keep for + // completeness. + // fail (ll) << "registered for cleanup wildcard " << p - << " doesn't match a directory"; + << (r == rmdir_status::not_empty + ? " matches the current directory" + : " doesn't match a directory"); + } - if (l.to_directory ()) + // Remove files or directories using wildcard. Removal of + // sub-entries of non-existing directory is not an error for + // 'maybe' cleanup type. + // + // Note that only the leaf part of the cleanup wildcard is a + // pattern in terms of libbutl::path_search(). + // + if (ls == "*" || ls == "**") { - auto rm = - [&p, &d, &ll] (path&& de, const string&, bool interm) -> bool + dir_path d (p.directory ()); + + if (t == cleanup_type::always && !dir_exists (d)) + fail (ll) << "registered for cleanup wildcard " << p + << " doesn't match a directory"; + + if (l.to_directory ()) { - if (!interm) + auto rm = + [&p, &d, &ll] (path&& de, const string&, bool interm) -> bool { - dir_path sd (path_cast (d / de)); + if (!interm) + { + dir_path sd (path_cast (d / de)); - // We can get not_exist here due to racing conditions, but - // that's ok if somebody did our job. - // - rmdir_status r (rmdir (sd, 2)); + // We can get not_exist here due to racing conditions, but + // that's ok if somebody did our job. + // + rmdir_status r (rmdir (sd, 2)); + + if (r != rmdir_status::not_empty) + return true; - if (r == rmdir_status::not_empty) fail (ll) << "registered for cleanup directory " << sd << " is not empty" << info << "wildcard: '" << p << "'"; - } + } - return true; - }; + return true; + }; - path_search (l, rm, d); - } - else - { - auto rm = [&d] (path&& p, const string&, bool interm) -> bool + path_search (l, rm, d); + } + else { - if (!interm) - rmfile (d / p, 2); // That's ok if not exists. + auto rm = [&d] (path&& p, const string&, bool interm) -> bool + { + if (!interm) + rmfile (d / p, 2); // That's ok if not exists. - return true; - }; + return true; + }; - path_search (l, rm, d); + path_search (l, rm, d); + } + + continue; } - continue; - } + // Remove the directory if exists and empty. Fail otherwise. + // Removal of non-existing directory is not an error for 'maybe' + // cleanup type. + // + if (p.to_directory ()) + { + dir_path d (path_cast (p)); - // Remove the directory if exists and empty. Fail otherwise. Removal - // of non-existing directory is not an error for 'maybe' cleanup - // type. - // - if (p.to_directory ()) - { - dir_path d (path_cast (p)); + // @@ If 'd' is a file then will fail with a diagnostics having + // no location info. Probably need to add an optional location + // parameter to rmdir() function. The same problem exists for + // a file cleanup when try to rmfile() directory instead of + // file. + // + rmdir_status r (rmdir (d, 2)); - // @@ If 'd' is a file then will fail with a diagnostics having no - // location info. Probably need to add an optional location - // parameter to rmdir() function. The same problem exists for a - // file cleanup when try to rmfile() directory instead of file. - // - rmdir_status r (rmdir (d, 2)); + if (r == rmdir_status::success || + (r == rmdir_status::not_exist && t == cleanup_type::maybe)) + continue; - if (r == rmdir_status::success || - (r == rmdir_status::not_exist && t == cleanup_type::maybe)) - continue; + fail (ll) << "registered for cleanup directory " << d + << (r == rmdir_status::not_empty + ? " is not empty" + : " does not exist"); + } - fail (ll) << "registered for cleanup directory " << d - << (r == rmdir_status::not_empty - ? " is not empty" - : " does not exist"); + // Remove the file if exists. Fail otherwise. Removal of + // non-existing file is not an error for 'maybe' cleanup type. + // + if (rmfile (p, 2) == rmfile_status::not_exist && + t == cleanup_type::always) + fail (ll) << "registered for cleanup file " << p + << " does not exist"; } - - // Remove the file if exists. Fail otherwise. Removal of non-existing - // file is not an error for 'maybe' cleanup type. - // - if (rmfile (p, 2) == rmfile_status::not_exist && - t == cleanup_type::always) - fail (ll) << "registered for cleanup file " << p - << " does not exist"; } // Return to the parent scope directory or to the out_base one for the @@ -1609,10 +1617,19 @@ namespace build2 } void default_runner:: - run (scope& sp, const command_expr& expr, size_t li, const location& ll) + run (scope& sp, + const command_expr& expr, command_type ct, + size_t li, + const location& ll) { + // Noop for teardown commands if keeping tests output is requested. + // + if (ct == command_type::teardown && + common_.after == output_after::keep) + return; + if (verb >= 3) - text << expr; + text << ct << expr; if (!run_expr (sp, expr, li, ll, true)) throw failed (); // Assume diagnostics is already printed. diff --git a/build2/test/script/script b/build2/test/script/script index 04c54db..c238daf 100644 --- a/build2/test/script/script +++ b/build2/test/script/script @@ -313,6 +313,13 @@ namespace build2 ostream& operator<< (ostream&, const command_expr&); + // command_type + // + enum class command_type {test, setup, teardown}; + + ostream& + operator<< (ostream&, command_type); + // description // struct description diff --git a/build2/test/script/script.cxx b/build2/test/script/script.cxx index c5cede8..0fa9dc6 100644 --- a/build2/test/script/script.cxx +++ b/build2/test/script/script.cxx @@ -305,6 +305,23 @@ namespace build2 } } + // command_type + // + ostream& + operator<< (ostream& o, command_type ct) + { + const char* s (nullptr); + + switch (ct) + { + case command_type::test: s = ""; break; + case command_type::setup: s = "+"; break; + case command_type::teardown: s = "-"; break; + } + + return o << s; + } + // redirect // redirect:: diff --git a/tests/test/script/runner/buildfile b/tests/test/script/runner/buildfile index c3df228..a990405 100644 --- a/tests/test/script/runner/buildfile +++ b/tests/test/script/runner/buildfile @@ -2,7 +2,7 @@ # copyright : Copyright (c) 2014-2017 Code Synthesis Ltd # license : MIT; see accompanying LICENSE file -./: test{cleanup expr if pipe redirect regex set status} exe{driver} $b +./: test{cleanup expr if output pipe redirect regex set status} exe{driver} $b test{*}: target = exe{driver} diff --git a/tests/test/script/runner/output.test b/tests/test/script/runner/output.test new file mode 100644 index 0000000..f46b6ec --- /dev/null +++ b/tests/test/script/runner/output.test @@ -0,0 +1,87 @@ +# file : tests/test/script/runner/output.test +# copyright : Copyright (c) 2014-2017 Code Synthesis Ltd +# license : MIT; see accompanying LICENSE file + +# Some of the tests below (*/script-wd, before/*) are probably more appropriate +# for tests/test/script/script-integration/testscript. Nevertheless let's not +# spread the feature tests among several places. +# + +.include ../common.test + +: after +: +{ + : keep + : + { + : cleanup + : + $c <'touch a' && $b config.test.output=keep; + test -f test/1/a + + : teardown + : + $c <>/EOE != 0 + error: working directory test/ exists at the beginning of the test + EOE + + : warn + : + mkdir test &!test/; + $c <'true' && $b config.test.output=warn@clean 2>>/EOE + warning: working directory test/ exists at the beginning of the test + EOE + + : clean + : + mkdir test &!test/; + $c <'true' && $b config.test.output=clean@clean +} diff --git a/unit-tests/test/script/parser/command-if.test b/unit-tests/test/script/parser/command-if.test index ffe2c35..29d1dce 100644 --- a/unit-tests/test/script/parser/command-if.test +++ b/unit-tests/test/script/parser/command-if.test @@ -89,7 +89,7 @@ EOI { ? true - cmd + +cmd } EOO @@ -102,7 +102,7 @@ EOI { ? true - cmd + -cmd } EOO } diff --git a/unit-tests/test/script/parser/driver.cxx b/unit-tests/test/script/parser/driver.cxx index 5872fb8..772bc10 100644 --- a/unit-tests/test/script/parser/driver.cxx +++ b/unit-tests/test/script/parser/driver.cxx @@ -90,11 +90,11 @@ namespace build2 virtual void run (scope&, - const command_expr& e, + const command_expr& e, command_type t, size_t i, const location&) override { - cout << ind_ << e; + cout << ind_ << t << e; if (line_) cout << " # " << i; diff --git a/unit-tests/test/script/parser/scope-if.test b/unit-tests/test/script/parser/scope-if.test index 4f524cf..2867107 100644 --- a/unit-tests/test/script/parser/scope-if.test +++ b/unit-tests/test/script/parser/scope-if.test @@ -489,7 +489,7 @@ else -tdown # 8 EOI { - setup # 1 + +setup # 1 ? false one # 2 ? false two # 3 ? true # 4 @@ -501,7 +501,7 @@ EOI { cmd2 # 0 } - tdown # 8 + -tdown # 8 } EOO diff --git a/unit-tests/test/script/parser/scope.test b/unit-tests/test/script/parser/scope.test index 144dc54..437a3dc 100644 --- a/unit-tests/test/script/parser/scope.test +++ b/unit-tests/test/script/parser/scope.test @@ -115,7 +115,7 @@ $* foo.test <'cmd "$~"' >~"%cmd '?.+[/\\\\]test-driver[/\\\\]foo[/\\\\]1'?%" EOI { { # 1 - setup + +setup { # 1/4 cmd abc } -- cgit v1.1