Keep timestamps off the stack

[Kacarott]

(I am not sure why this this wasn't raised as an issue, and have just remembered it now, so moving it to a more appropriate place).
Currently, both it#{ and describe#{ leave a timestamp on the stack, used for reporting the runtime of the users code. This is essentially an implementation detail, but the issue is that it can interfere with user code. Consider this:

"tests" describe#{
   <some-large-data-structure>
   "should work in context A" it#{
     dup run-test-a
   }#
  
   "should work in context B" it#{
     dup run-test-b
   }#
  drop
}#

This seems fairly regular code, however without the author realising it, it will be calling both run-test words with some arbitrary number, instead of the expected data structure!

There is really no reason for the timestamps to be left on the stack like this, and it is simple to fix. Simply scan the relevant block, and run the resulting quotation inside dip such that the timestamp is moved to the retain stack. See this for a simple proof of concept.

[nomennescio]

I want to clarify this is not regular code; the whole idea behind it#{..} blocks is that they isolate the code inside the block from the code outside, including all exceptions. It is by design that arguments are not the be passed to it#{..} blocks via the stack and that these code blocks don't leave an unbalanced stack.

However, I will give this some thought and will come back to this. I first want to get 0.99 working.

[Kacarott]

It may not be a regular example, however the it#{ blocks do not enforce code isolation, and if code isolation is not enforced then authors may run into this kind of issue, so where possible the implementation details should be hidden.

Even without it#{ blocks interacting with outside code, there is weird and unexpected behaviours. A user solution using stack. for debugging will see an unexplained number as the first element. Buggy test code which mis-juggles stack elements could result in a number used as an assertion message, only to fail when trying to subtract a string from a number, rather than the expected stack error.

If code isolation is what you intended, I can fairly easily add a stack-effect check to my existing PR, to ensure that all it#{ and describe#{ blocks have an effect of ( -- )?

[Kacarott]

Considering that this is an entirely non-breaking change, I had hoped to get the PR merged before the 0.99 update as it will likely be more involved by then.

[nomennescio]

There's no rush, I've made new releases of the test framework before. If I would make this change, I'd rather combine it with some other issues I would like to solve.

[Kacarott]

Well that would be why I would suggest to get it resolved before the 0.99 release, since that release is inevitable anyway. So perhaps can you explain your hesitance?

There's no rush

Yes I am aware, but recall that we first discussed this 5 months ago when you directed me to post it on Github discussions for further discussion and consideration, but when that didnt happen I thought I may as well contribute the fix myself, considering how often I use the vocab :)

[nomennescio]

Sorry, missed that you posted it as a discussion in here. My hesitance is related to what I stated; it's not in line with the original design, which is also used by the Forth test framework. I think you're probably the only one wanting to use the test framework like this, and I've noticed you're using the test framework in unexpected ways. Before going there I want to make sure things keep working like they were intended.

[Kacarott]

I still don't see how it isn't inline with the original design, the it and describe blocks are still run the same as before, the only change is that the timestamp value is moved to the retain stack instead of the call stack. Currently we have something like (simplified)
nano-count "<IT::>" print ...assertion code... nano-count swap - "<COMPLETEDIN::>%f ms" printf
And the suggested new code is simply changing it to
nano-count "<IT::>" print [ ...assertion code... ] dip nano-count swap - "<COMPLETEDIN::>%f ms" printf.

I think you're probably the only one wanting to use the test framework like this

I think you are misunderstanding me. This is not a feature request so that I can use the framework in weird ways, this is a bug report and fix due to strange behaviour, both for solvers and authors. I had noticed it as a solver before but ignored it, but it was actually gifti who brought it up as a bug they had encountered while authoring, it was just me who felt motivated to solve it.