| Message ID | pull.1374.git.1665068476.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | tests: add test_todo() for known failures | expand |
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes: > test_todo() is intended as a fine grained alternative to > test_expect_failure(). Rather than marking the whole test as failing > test_todo() is used to mark individual failing commands within a test. This > approach to writing failing tests allows us to detect unexpected failures > that are hidden by test_expect_failure(). > > This series attempts to keep most of the benefits test_expect_todo() > previously proposed by Ævar[1] while being simpler to use. Great. We discussed this some time ago and I am happy to see the work re-ignited. Thanks.
On Thu, Oct 06 2022, Phillip Wood via GitGitGadget wrote: > test_todo() is intended as a fine grained alternative to > test_expect_failure(). Rather than marking the whole test as failing > test_todo() is used to mark individual failing commands within a test. This > approach to writing failing tests allows us to detect unexpected failures > that are hidden by test_expect_failure(). > > This series attempts to keep most of the benefits test_expect_todo() > previously proposed by Ævar[1] while being simpler to use. > > [1] > https://lore.kernel.org/git/cover-0.7-00000000000-20220318T002951Z-avarab@gmail.com/ I like the interface you've got here much better than the one I submitted in [1], so much that it's what I tried to write at first :) But as you noted in 1/3: test_todo cannot be used in a subshell. So when we do this: diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 93d3930d9f6..75b84a09592 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -147,7 +147,7 @@ test_expect_success 'subtest: a failing test_todo' ' false } test_expect_success "passing test" "true" - test_expect_success "known todo" "test_todo test_false" + test_expect_success "known todo" "(test_todo test_false)" test_done EOF check_sub_test_lib_test failing-test-todo <<-\EOF We'll get: + diff -u failing-test-todo/expect.out failing-test-todo/out --- failing-test-todo/expect.out 2022-10-06 19:30:14.093338392 +0000 +++ failing-test-todo/out 2022-10-06 19:30:14.093338392 +0000 @@ -1,5 +1,4 @@ ok 1 - passing test -not ok 2 - known todo # TODO known breakage -# still have 1 known breakage(s) -# passed all remaining 1 test(s) +ok 2 - known todo +# passed all 2 test(s) 1..2 What I was initially trying to do when I tried this approach was to make the "test_todo" be the equivalent of a sub-test, i.e. when we encounter one we'd say "ok N - DESC" for the current test so far, and then an "ok N+1 - DESC # TODO: $cmd" for the "test_todo" command. I think I lost the code for that, but I tried hacking something rough up on top of your series. I don't think it's a viable approach but it works as long as we don't have a subshell (the "remaining N tests" count is off, but it's fixable, I just couldn't be bothered for a WIP hack): diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 93d3930d9f6..7e8e0a54558 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -148,14 +148,17 @@ test_expect_success 'subtest: a failing test_todo' ' } test_expect_success "passing test" "true" test_expect_success "known todo" "test_todo test_false" + test_expect_success "passing test 2" "true" test_done EOF check_sub_test_lib_test failing-test-todo <<-\EOF > ok 1 - passing test - > not ok 2 - known todo # TODO known breakage + > not ok 2 - known todo: test_false # TODO known breakage + > ok 3 - known todo (post-test_todo) + > ok 4 - passing test 2 > # still have 1 known breakage(s) - > # passed all remaining 1 test(s) - > 1..2 + > # passed all remaining 3 test(s) + > 1..4 EOF ' @@ -171,7 +174,8 @@ test_expect_success 'subtest: a passing test_todo' ' check_sub_test_lib_test passing-test-todo <<-\EOF > not ok 1 - pretend we have fixed a test_todo breakage > # test_todo test_true - > # failed 1 among 1 test(s) + > # 1 known breakage(s) vanished; please update test(s) + > # failed 1 among remaining 0 test(s) > 1..1 EOF ' diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 068a0702809..54365fe202f 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -826,15 +826,12 @@ test_expect_success () { then test -n "$test_skip_test_preamble" || say >&3 "expecting success of $TEST_NUMBER.$test_count '$1': $2" - test_todo_=test_expect_success + test_todo_title_="$1" + test_had_todo_= if test_run_ "$2" then - if test "$test_todo_" = "todo" - then - test_known_broken_failure_ "$1" - else - test_ok_ "$1" - fi + test_ok_ "$1${test_had_todo_:+ (post-test_todo)}" + test_had_todo_= else test_failure_ "$@" fi @@ -1167,12 +1164,26 @@ test_must_fail_helper () { # "test_*" assertions such as test_cmp(). test_todo () { + if test -z "$test_todo_title_" + then + BUG 'test_todo: expected a $test_todo_title_' + fi && if test "$test_todo_" = "test_expect_failure" then BUG "test_todo_ cannot be used inside test_expect_failure" + fi && + # Tell "test_expect_success" it had a "test_todo" + test_had_todo_=1 && + # We say that the test up until this point is OK, and emit an "ok .." for it. + test_ok_ "$test_todo_title_" && + if test_must_fail_helper todo "$@" 2>&7 + then + test_known_broken_failure_ "$test_todo_title_: $*" 1>&5 2>&6 && + test_count=$(($test_count+1)) + else + test_known_broken_ok_ "$test_todo_title_: $*" && + return 1 fi - test_todo_=todo - test_must_fail_helper todo "$@" 2>&7 } 7>&2 2>&4 # This is not among top-level (test_expect_success | test_expect_failure) Anyway, the core difference between the APIs we proposed for this is that you'd do: test_expect_success 'desc' 'test_todo false' Whereas I suggested: test_expect_todo 'desc' '! false' Now, let's pick apart the differences: 1. With "test_expect_todo" we're declaring "this is a TODO test" for the test as a whole. 2. With your "test_todo" we're not doing that, instead we proceed as normal, and then we might note "we had a TODO" midway through, then at the end we'll spot that we had a TODO test (but this approach won't work with subshells). 3. Your "test_todo" is basically a "let's let this pass", whereas mine was a helper which exhaustively declared *what* the bad behavior was. (Although some of yours seems to be midway between the two, i.e. https://lore.kernel.org/git/c3f4a79c-2dc6-fbf4-fc61-591ebf417682@dunelm.org.uk/) I think the main critique you and Junio had of my series was to do with #3, i.e. that it was a hassle to exhaustively declare what the behavior is & should be, as you note in: https://lore.kernel.org/git/c3f4a79c-2dc6-fbf4-fc61-591ebf417682@dunelm.org.uk/ test_todo \ --want "test_must_fail git" \ --reset "git reset --hard" \ --expect git \ -- \ rm d/f && That's fair enough, maybe that's not worth the effort. The reason I initially hacked this up was because I'd noticed a behavior difference in a command that was only revealed in a test_expect_failure block, but because we didn't assert *what* the behavior was we didn't notice. My version (if fully used) would spot that, but that's because of how I wrote the "tes_todo", it's orthagonal to #1 and #2 above. So I don't see why we wouldn't instead have a "test_expect_todo" and just write the helper differently, or have a mode where it's less strict, and (if we find it worthwhile) one where it's more strict. I rebased my https://lore.kernel.org/git/patch-1.7-4624abc2591-20220318T002951Z-avarab@gmail.com/ just now and applied the below on top, which seems to me to give you pretty much the end result you want, the only difference is that my version will also work in subshells (see the t2500 one): diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index de1ec89007d..fe47e503bd1 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -468,7 +468,7 @@ test_expect_success 'sparse-checkout (init|set|disable) warns with unmerged stat git -C unmerged sparse-checkout disable ' -test_expect_failure 'sparse-checkout reapply' ' +test_expect_todo 'sparse-checkout reapply' ' git clone repo tweak && echo dirty >tweak/deep/deeper2/a && @@ -502,11 +502,11 @@ test_expect_failure 'sparse-checkout reapply' ' # NEEDSWORK: We are asking to update a file outside of the # sparse-checkout cone, but this is no longer allowed. - git -C tweak add folder1/a && + test_todo git -C tweak add folder1/a && git -C tweak sparse-checkout reapply 2>err && - test_must_be_empty err && + test_todo test_must_be_empty err && test_path_is_missing tweak/deep/deeper2/a && - test_path_is_missing tweak/folder1/a && + test_todo test_path_is_missing tweak/folder1/a && git -C tweak sparse-checkout disable ' diff --git a/t/t2500-untracked-overwriting.sh b/t/t2500-untracked-overwriting.sh index 5c0bf4d21fc..db7c72d38d8 100755 --- a/t/t2500-untracked-overwriting.sh +++ b/t/t2500-untracked-overwriting.sh @@ -167,7 +167,7 @@ test_expect_success 'git rebase fast forwarding and untracked files' ' ) ' -test_expect_failure 'git rebase --autostash and untracked files' ' +test_expect_todo 'git rebase --autostash and untracked files' ' test_setup_sequencing rebase_autostash_and_untracked && ( cd sequencing_rebase_autostash_and_untracked && @@ -176,7 +176,7 @@ test_expect_failure 'git rebase --autostash and untracked files' ' mkdir filler && echo precious >filler/file && cp filler/file expect && - git rebase --autostash init && + test_todo git rebase --autostash init && test_path_is_file filler/file ) ' diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index 3b0fa66c33d..b31b6b0f7a0 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -577,7 +577,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' ' grep "cherry picked from.*$picked" msg ' -test_expect_failure '--signoff is automatically propagated to resolved conflict' ' +test_expect_todo '--signoff is automatically propagated to resolved conflict' ' pristine_detach initial && test_expect_code 1 git cherry-pick --signoff base..anotherpick && echo "c" >foo && @@ -591,7 +591,7 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict' git cat-file commit HEAD~3 >initial_msg && ! grep "Signed-off-by:" initial_msg && grep "Signed-off-by:" unrelatedpick_msg && - ! grep "Signed-off-by:" picked_msg && + test_todo ! grep "Signed-off-by:" picked_msg && grep "Signed-off-by:" anotherpick_msg ' diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index e74a318ac33..6c7929f5557 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -790,7 +790,7 @@ test_expect_success SYMLINKS 'rm across a symlinked leading path (no index)' ' test_path_is_file e/f ' -test_expect_failure SYMLINKS 'rm across a symlinked leading path (w/ index)' ' +test_expect_todo SYMLINKS 'rm across a symlinked leading path (w/ index)' ' rm -rf d e && mkdir d && echo content >d/f && @@ -798,10 +798,10 @@ test_expect_failure SYMLINKS 'rm across a symlinked leading path (w/ index)' ' git commit -m "d/f exists" && mv d e && ln -s e d && - test_must_fail git rm d/f && - git rev-parse --verify :d/f && + test_todo test_must_fail git rm d/f && + test_todo git rev-parse --verify :d/f && test -h d && - test_path_is_file e/f + test_todo test_path_is_file e/f ' test_expect_success 'setup for testing rm messages' ' diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index ad5c0292794..a6a5a330180 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -165,12 +165,12 @@ test_expect_success 'additional command line cc (ascii)' ' grep "^ *S E Cipient <scipient@example.com>\$" hdrs5 ' -test_expect_failure 'additional command line cc (rfc822)' ' +test_expect_todo 'additional command line cc (rfc822)' ' git config --replace-all format.headers "Cc: R E Cipient <rcipient@example.com>" && git format-patch --cc="S. E. Cipient <scipient@example.com>" --stdout main..side >patch5 && sed -e "/^\$/q" patch5 >hdrs5 && grep "^Cc: R E Cipient <rcipient@example.com>,\$" hdrs5 && - grep "^ *\"S. E. Cipient\" <scipient@example.com>\$" hdrs5 + test_todo grep "^ *\"S. E. Cipient\" <scipient@example.com>\$" hdrs5 ' test_expect_success 'command line headers' ' diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index f342954de11..9d5706454a5 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1049,6 +1049,21 @@ test_must_fail_acceptable () { esac } +test_todo () { + local negate=-ne + local cmp_op=-ne + if test "$1" = "!" + then + negate=t && + cmp_op=-eq + shift + fi && + "$@" 2>&7 + exit_code=$? + say "test_todo: got $exit_code ${negate:+negated!} from $*" + test "$exit_code" "$cmp_op" 0 +} + # This is not among top-level (test_expect_success | test_expect_failure) # but is a prefix that can be used in the test script, like: #
Hi Ævar On 06/10/2022 20:28, Ævar Arnfjörð Bjarmason wrote: > > On Thu, Oct 06 2022, Phillip Wood via GitGitGadget wrote: > >> test_todo() is intended as a fine grained alternative to >> test_expect_failure(). Rather than marking the whole test as failing >> test_todo() is used to mark individual failing commands within a test. This >> approach to writing failing tests allows us to detect unexpected failures >> that are hidden by test_expect_failure(). >> >> This series attempts to keep most of the benefits test_expect_todo() >> previously proposed by Ævar[1] while being simpler to use. >> >> [1] >> https://lore.kernel.org/git/cover-0.7-00000000000-20220318T002951Z-avarab@gmail.com/ > > I like the interface you've got here much better than the one I > submitted in [1], so much that it's what I tried to write at first :) > > But as you noted in 1/3: > > test_todo cannot be used in a subshell. > > Anyway, the core difference between the APIs we proposed for this is > that you'd do: > > test_expect_success 'desc' 'test_todo false' > > Whereas I suggested: > > test_expect_todo 'desc' '! false' > > Now, let's pick apart the differences: > > 1. With "test_expect_todo" we're declaring "this is a TODO test" for > the test as a whole. > > 2. With your "test_todo" we're not doing that, instead we proceed as > normal, and then we might note "we had a TODO" midway through, then > at the end we'll spot that we had a TODO test (but this approach > won't work with subshells). Yes, this series avoids adding test_expect_todo and reuses test_expect_success as Junio suggested [1]. By using a new toplevel form your series was able to handle test_todo in a subshell because it did not need any global state related to whether a test_todo had passed/failed. The series here uses a variable to check if any test_todo statements were present in a test that ran successfully and that does not work if the test_todo happens in a subshell because the variable in the parent is not updated. First we need to consider whether there is any need for supporting test_todo in a subshell. If there is then a possible alternative is to store the state in a file. That would add the cost of "test -f" to test_expect_success but our tests do so much i/o that a single extra stat should not be noticeable. In particular I believe a file based approach can be implemented without adding any new processes to tests that do not use test_todo. > 3. Your "test_todo" is basically a "let's let this pass", whereas mine > was a helper which exhaustively declared *what* the bad behavior > was. > > (Although some of yours seems to be midway between the two, > i.e. https://lore.kernel.org/git/c3f4a79c-2dc6-fbf4-fc61-591ebf417682@dunelm.org.uk/) The only thing my series tries to assert is that a test_todo command is not failing due to a usage error so that it can catch buggy tests. Beyond that it does not care what the reason for failure is. > I think the main critique you and Junio had of my series was to do with > #3, i.e. that it was a hassle to exhaustively declare what the behavior > is & should be, as you note in: That was certainly my main objection, but Junio was not that keen on adding test_expect_todo [1] > https://lore.kernel.org/git/c3f4a79c-2dc6-fbf4-fc61-591ebf417682@dunelm.org.uk/ > > test_todo \ > --want "test_must_fail git" \ > --reset "git reset --hard" \ > --expect git \ > -- \ > rm d/f && > > That's fair enough, maybe that's not worth the effort. The reason I > initially hacked this up was because I'd noticed a behavior difference > in a command that was only revealed in a test_expect_failure block, but > because we didn't assert *what* the behavior was we didn't notice. > > My version (if fully used) would spot that, but that's because of how I > wrote the "tes_todo", it's orthagonal to #1 and #2 above. > > So I don't see why we wouldn't instead have a "test_expect_todo" and > just write the helper differently, or have a mode where it's less > strict, and (if we find it worthwhile) one where it's more strict. I think there is a question of whether we need a new toplevel test_expect_todo - why would we add it if we can just reuse test_expect_success? That way when a test failure is fixed all that needs to be done is to remove the test_todo calls. Best Wishes Phillip [1] https://lore.kernel.org/git/xmqq8rt77zp7.fsf@gitster.g/ > I rebased my > https://lore.kernel.org/git/patch-1.7-4624abc2591-20220318T002951Z-avarab@gmail.com/ > just now and applied the below on top, which seems to me to give you > pretty much the end result you want, the only difference is that my > version will also work in subshells (see the t2500 one): > > diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh > index de1ec89007d..fe47e503bd1 100755 > --- a/t/t1091-sparse-checkout-builtin.sh > +++ b/t/t1091-sparse-checkout-builtin.sh > @@ -468,7 +468,7 @@ test_expect_success 'sparse-checkout (init|set|disable) warns with unmerged stat > git -C unmerged sparse-checkout disable > ' > > -test_expect_failure 'sparse-checkout reapply' ' > +test_expect_todo 'sparse-checkout reapply' ' > git clone repo tweak && > > echo dirty >tweak/deep/deeper2/a && > @@ -502,11 +502,11 @@ test_expect_failure 'sparse-checkout reapply' ' > > # NEEDSWORK: We are asking to update a file outside of the > # sparse-checkout cone, but this is no longer allowed. > - git -C tweak add folder1/a && > + test_todo git -C tweak add folder1/a && > git -C tweak sparse-checkout reapply 2>err && > - test_must_be_empty err && > + test_todo test_must_be_empty err && > test_path_is_missing tweak/deep/deeper2/a && > - test_path_is_missing tweak/folder1/a && > + test_todo test_path_is_missing tweak/folder1/a && > > git -C tweak sparse-checkout disable > ' > diff --git a/t/t2500-untracked-overwriting.sh b/t/t2500-untracked-overwriting.sh > index 5c0bf4d21fc..db7c72d38d8 100755 > --- a/t/t2500-untracked-overwriting.sh > +++ b/t/t2500-untracked-overwriting.sh > @@ -167,7 +167,7 @@ test_expect_success 'git rebase fast forwarding and untracked files' ' > ) > ' > > -test_expect_failure 'git rebase --autostash and untracked files' ' > +test_expect_todo 'git rebase --autostash and untracked files' ' > test_setup_sequencing rebase_autostash_and_untracked && > ( > cd sequencing_rebase_autostash_and_untracked && > @@ -176,7 +176,7 @@ test_expect_failure 'git rebase --autostash and untracked files' ' > mkdir filler && > echo precious >filler/file && > cp filler/file expect && > - git rebase --autostash init && > + test_todo git rebase --autostash init && > test_path_is_file filler/file > ) > ' > diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh > index 3b0fa66c33d..b31b6b0f7a0 100755 > --- a/t/t3510-cherry-pick-sequence.sh > +++ b/t/t3510-cherry-pick-sequence.sh > @@ -577,7 +577,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' ' > grep "cherry picked from.*$picked" msg > ' > > -test_expect_failure '--signoff is automatically propagated to resolved conflict' ' > +test_expect_todo '--signoff is automatically propagated to resolved conflict' ' > pristine_detach initial && > test_expect_code 1 git cherry-pick --signoff base..anotherpick && > echo "c" >foo && > @@ -591,7 +591,7 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict' > git cat-file commit HEAD~3 >initial_msg && > ! grep "Signed-off-by:" initial_msg && > grep "Signed-off-by:" unrelatedpick_msg && > - ! grep "Signed-off-by:" picked_msg && > + test_todo ! grep "Signed-off-by:" picked_msg && > grep "Signed-off-by:" anotherpick_msg > ' > > diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh > index e74a318ac33..6c7929f5557 100755 > --- a/t/t3600-rm.sh > +++ b/t/t3600-rm.sh > @@ -790,7 +790,7 @@ test_expect_success SYMLINKS 'rm across a symlinked leading path (no index)' ' > test_path_is_file e/f > ' > > -test_expect_failure SYMLINKS 'rm across a symlinked leading path (w/ index)' ' > +test_expect_todo SYMLINKS 'rm across a symlinked leading path (w/ index)' ' > rm -rf d e && > mkdir d && > echo content >d/f && > @@ -798,10 +798,10 @@ test_expect_failure SYMLINKS 'rm across a symlinked leading path (w/ index)' ' > git commit -m "d/f exists" && > mv d e && > ln -s e d && > - test_must_fail git rm d/f && > - git rev-parse --verify :d/f && > + test_todo test_must_fail git rm d/f && > + test_todo git rev-parse --verify :d/f && > test -h d && > - test_path_is_file e/f > + test_todo test_path_is_file e/f > ' > > test_expect_success 'setup for testing rm messages' ' > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index ad5c0292794..a6a5a330180 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -165,12 +165,12 @@ test_expect_success 'additional command line cc (ascii)' ' > grep "^ *S E Cipient <scipient@example.com>\$" hdrs5 > ' > > -test_expect_failure 'additional command line cc (rfc822)' ' > +test_expect_todo 'additional command line cc (rfc822)' ' > git config --replace-all format.headers "Cc: R E Cipient <rcipient@example.com>" && > git format-patch --cc="S. E. Cipient <scipient@example.com>" --stdout main..side >patch5 && > sed -e "/^\$/q" patch5 >hdrs5 && > grep "^Cc: R E Cipient <rcipient@example.com>,\$" hdrs5 && > - grep "^ *\"S. E. Cipient\" <scipient@example.com>\$" hdrs5 > + test_todo grep "^ *\"S. E. Cipient\" <scipient@example.com>\$" hdrs5 > ' > > test_expect_success 'command line headers' ' > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh > index f342954de11..9d5706454a5 100644 > --- a/t/test-lib-functions.sh > +++ b/t/test-lib-functions.sh > @@ -1049,6 +1049,21 @@ test_must_fail_acceptable () { > esac > } > > +test_todo () { > + local negate=-ne > + local cmp_op=-ne > + if test "$1" = "!" > + then > + negate=t && > + cmp_op=-eq > + shift > + fi && > + "$@" 2>&7 > + exit_code=$? > + say "test_todo: got $exit_code ${negate:+negated!} from $*" > + test "$exit_code" "$cmp_op" 0 > +} > + > # This is not among top-level (test_expect_success | test_expect_failure) > # but is a prefix that can be used in the test script, like: > # > >
Phillip Wood <phillip.wood123@gmail.com> writes: > I think there is a question of whether we need a new toplevel > test_expect_todo - why would we add it if we can just reuse > test_expect_success? That way when a test failure is fixed all that > needs to be done is to remove the test_todo calls. Yup, that is one of the reasons why I favor test_todo inside the normal test_expect_success. A patch that fixes a breakage would come with a change to the tests that flips test_expect_failure to test_expect_success often had the step that were expected to fail outside the post context and did not make it immediately obvious what was fixed, and use of a more focused test_todo would make it more clear. Unless we gain a clear advantage by using a different top-level (e.g. some of the limitations like "not in a subshell" can be lifted?), I do not think we want to add one.