-
Notifications
You must be signed in to change notification settings - Fork 328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make test failure source locations lazy #12260
base: develop
Are you sure you want to change the base?
Changes from all commits
12632d6
007f068
c4438e7
f926e8b
461f7b2
39bcd9b
5c5a9bd
1fd5911
ff9c67a
ca2c0ce
10e6212
8f26d0a
20072e9
b8afddf
75cfe92
10814db
c3635b0
c4cdca9
7fff45f
11ba0f8
429fd69
2cdfdf9
e9d78e3
c8af61b
9828302
e524998
0eb6018
02dd307
26ed0f6
be6f652
9c96e13
2725dc9
023bb2b
0661925
5fe1222
635e050
903a364
de153c1
c9db11c
3e1f4d2
fc7a125
b7d9d60
9822e59
b2140bb
88115ce
049f263
b334d28
551fa7b
f0cdd00
cf7bc55
eb0553e
260c482
dec480c
cc3f27d
9e64f85
4b77e2b
496c2bd
c75bc3f
d1bed32
d3e668c
82b40c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
from Standard.Base import all | ||
import Standard.Base.Runtime.Ref.Ref | ||
|
||
## PRIVATE | ||
|
||
Frame_Hider is used in test utilities to hide uninteresting stack frames from | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name Do you have some prior art for selecting such a name? When thinking about an alternative a phase shadow stack come to my mind:
Maybe "shadow stack" would be misleading as well, but "to shadow" certainly feels more positive than "to hide" ;-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if this is shadowing -- that implies a duplicate / parallel stack. What we are really doing is removing some frames from the stack before picking the top frame. I originally called this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be honest looking at all the possible names, It is an internal API anyway - mostly for library developers so the naming can probably be a bit less stringent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Internal API would be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But it is less public than GUI-facing things. It is marked as PRIVATE and meant to be used by library developers (advanced users) not by every user. There is some difference. |
||
users when reporting the location of test failures. | ||
|
||
In a test utility such as `Any.should_equal`, the failure of a test assertion | ||
is accompanied by the location of the failure. The location should not be a | ||
stack frame inside `Any.should_equal`, but rather should be at the location | ||
that called `Any.should_equal`. This means identifying stack frames we don't | ||
want to bother the user with, and omitting them when we're looking for the | ||
correct stack frame to use. | ||
|
||
The problem is more complicated when test utilities call other utilities, or | ||
when test utilities invoke callbacks provided by user test code. | ||
|
||
The `hide` and `unhide*` methods are simple wrappers. `get_top_stack_frame` | ||
identifies these on the stack, and uses them to remove the uninteresting | ||
frames from consderation. | ||
|
||
A `hide` call should be wrapped around the body of every test utility. | ||
|
||
An `unhide*` call should be wrapped around any call from a test utility to a | ||
user-supplied callback, depending on the number of arguments. If one test | ||
utility forwards a callback to another test utility, the callback should be | ||
wrapped at the forward site as well. | ||
|
||
! Partial Application | ||
|
||
Partial application can prevent the `unhide*` methods from existing in the | ||
stack trace, so test callbacks should have explicit parameters: | ||
|
||
# Do not use partial application: | ||
tester = expect_column_names ["A", "B"] | ||
# Use explicit parameters: | ||
tester t = expect_column_names ["A", "B"] t | ||
! Tail Calls | ||
|
||
Tail calls can result in changes to the stack that prevent | ||
`get_top_stack_frame` from working properly. For this reason, uses of | ||
`hide` and `unhide` should be within the tail call: | ||
|
||
go i = Frame_Hider.hide <| | ||
# ... | ||
@Tail_Call go (i+1) | ||
go 1 | ||
type Frame_Hider | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strangely, I am getting this error when I try this, but only in some tests -- other tests run correctly.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unexpected panic is an suspicious outcome - consider reporting it as a separate issue. If there is supposed to be an error, it shouldn't be unexpected panic. But yeah, such situations happen when we tease the compiler with unusual coding approaches. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After this PR, I'll create a task to remove the type and if it is still failing I'll report the bug. |
||
## PRIVATE | ||
|
||
Hides the stack frames of the contained code from `get_top_stack_frame`. | ||
|
||
> Example | ||
Wrap the body of a test utility to hide its stack frames. | ||
|
||
Any.should_equal self that = Frame_Hider.hide <| | ||
... | ||
Error.throw ("Not equal at " + Frame_Hider.get_top_stack_frame) | ||
hide ~action = | ||
action | ||
|
||
## PRIVATE | ||
|
||
Unhides the stack frames of the call to the user-supplied callback | ||
application. | ||
|
||
> Example | ||
Wrap a callback invocation to unhide its stack frames. | ||
|
||
test_utility test_predicate x = | ||
... | ||
Frame_Hider.unhide (test_predicate x) | ||
unhide ~action = | ||
action | ||
|
||
## PRIVATE | ||
|
||
Unhides the stack frames of the call to the user-supplied callback | ||
application, uncurried for one argument. | ||
|
||
`unhide_1 f x` is Equivalent to `unhide (f x)`. | ||
|
||
> Example | ||
Wrap a callback invocation to unhide its stack frames. | ||
|
||
test_utility test_predicate x = | ||
... | ||
Frame_Hider.unhide_1 test_predicate x | ||
unhide_1 f x = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, this is so non-FPish! I mean:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course I wanted to curry it, but it seems not to work; perhaps there is something I'm doing wrong. Demonstrated below: for a two-argument function, only the version with two explicit arguments has
Results:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In addition to this unfortunate code, I have had to do this in some tests that use callbacks:
( It makes me sad, but it was necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following program: from Standard.Base import all
sp = IO.println
foo a b =
st = Runtime.get_stack_trace
st.map IO.println
IO.println "===="
a + b
wrap_l ~a =
a
main =
sp <| foo 1 2
sp <| wrap_l <| foo 1 2
sp <| wrap_l <| foo 1 2
sp <| wrap_l <| foo 1 2 does include
The only difference is that one has to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But for a callback,
|
||
f x | ||
|
||
## PRIVATE | ||
|
||
Unhides the stack frames of the call to the user-supplied callback | ||
application, uncurried for two arguments. | ||
|
||
`unhide_1 f x y` is Equivalent to `unhide (f x y)`. | ||
|
||
> Example | ||
Wrap a callback invocation to unhide its stack frames. | ||
|
||
test_utility test_predicate x y = | ||
... | ||
Frame_Hider.unhide_2 test_predicate x y | ||
unhide_2 f x y = | ||
f x y | ||
|
||
## PRIVATE | ||
|
||
Identify the correct top stack frame by skipping over the stack frames | ||
inside test utilities. | ||
get_top_stack_frame -> Text = | ||
stack_trace = Runtime.get_stack_trace | ||
reversed_stack_trace = stack_trace.reverse | ||
|
||
## Iterate through stack frames from bottom to top, determining how many | ||
`hide` calls each one is nested inside. Each `hide` frame increases | ||
the nesting level by 1, and each `unhide` decreases it by one. | ||
|
||
The stack frame we are looking for is the one before the last frame | ||
that has a nesting level of 0 -- this is the most recent user-level | ||
call, and the one that triggered the test assertion failure. | ||
|
||
If we cannot find such a frame, we simply return the top of the | ||
stack. (In this case, some test code is missing a call to `unhide*`.) | ||
delta_for_frame frame = | ||
name = frame.name | ||
if name.starts_with "Frame_Hider.hide" then 1 else | ||
if name.starts_with "Frame_Hider.unhide" then -1 else 0 | ||
nesting_levels = reversed_stack_trace.map delta_for_frame . running_fold 0 (+) | ||
index = | ||
i = nesting_levels.last_index_of 0 | ||
if i.is_nothing || i == 0 then 0 else i-1 | ||
|
||
top = reversed_stack_trace . at index . source_location | ||
top.to_display_text |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,8 @@ export project.Extensions.should_succeed | |
|
||
export project.Faker.Faker | ||
|
||
export project.Frame_Hider.Frame_Hider | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ech, probably the |
||
|
||
export project.Problems | ||
|
||
export project.Suite.Suite | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this module be
private
? Probably it couldn't be.