assert_eventually(predicate_function, ...) #609
Conversation
There was a problem hiding this comment.
Sorry for the delay, been a busy week. Great start. I'm not sure how I feel about having to relearn my own code in reviewing changes, but I've sen a ton of things I've done that I don't like, heh.
Besides the comments I've added, add the following tests to test_awaiter.gd and get the failing ones to pass
func test_did_last_wait_timeout_is_false_by_default():
var a = add_child_autoqfree(Awaiter.new())
assert_false(a.did_last_wait_timeout)
func test_did_last_wait_timeout_is_readonly():
var a = add_child_autoqfree(Awaiter.new())
a.did_last_wait_timeout = true
assert_false(a.did_last_wait_timeout)
func test_wait_for_sets_did_timeout_to_true():
var a = add_child_autoqfree(Awaiter.new())
a.wait_for(.2)
await a.timeout
assert_true(a.did_last_wait_timeout)
func test_wait_for_resets_did_timeout():
var a = add_child_autoqfree(Awaiter.new())
a.wait_for(.2)
await a.timeout
a.wait_for(20)
assert_false(a.did_last_wait_timeout)
func test_wait_frames_sets_did_timeout_to_true():
var a = add_child_autoqfree(Awaiter.new())
a.wait_frames(10)
await a.timeout
assert_true(a.did_last_wait_timeout)
func test_wait_frames_resets_did_timeout():
var a = add_child_autoqfree(Awaiter.new())
a.wait_frames(10)
await a.timeout
a.wait_frames(50)
assert_false(a.did_last_wait_timeout)
func test_wait_for_signal_sets_did_timeout_to_true_when_time_exceeded():
var a = add_child_autoqfree(Awaiter.new())
var s = Signaler.new()
a.wait_for_signal(s.the_signal, .5)
assert_true(a.did_last_wait_timeout)
func test_wait_for_signal_results_in_false_timout():
var a = add_child_autoqfree(Awaiter.new())
a.wait_for(.2)
await a.timeout # results in true timeout
var s = Signaler.new()
a.wait_for_signal(s.the_signal, 10)
await get_tree().create_timer(.1).timeout
assert_false(a.did_last_wait_timeout)| @@ -22,14 +27,19 @@ func _physics_process(delta): | |||
| if(_elapsed_frames >= _wait_frames): | |||
| _end_wait() | |||
|
|
|||
| if(_predicate_function_waiting_to_be_true && _predicate_function_waiting_to_be_true.call()): | |||
There was a problem hiding this comment.
use and instead of && (per GDScript style guide).
|
|
||
| func _end_wait(): | ||
| _did_last_wait_timeout = _wait_time && _elapsed_time > _wait_time |
| the_signal.connect(_signal_callback) | ||
| _signal_to_wait_on = the_signal | ||
| _wait_time = x | ||
| wait_started.emit() | ||
|
|
||
| func wait_until(predicate_function: Callable, timeout_seconds): | ||
| _predicate_function_waiting_to_be_true = predicate_function | ||
| wait_for(timeout_seconds) |
There was a problem hiding this comment.
I prefer that this does not call wait_for. I'd rather see this method explicitly set the variables and emit the signal.
|
|
||
| func test_wait_until_reaches_timeout_when_predicate_function_never_returns_true(): | ||
| var node = add_child_autoqfree(Node.new()) | ||
| var is_named_foo = func(): return node.name == 'foo' |
There was a problem hiding this comment.
If this always returns false, it's a little easier to understand the test:
var is_named_foo = func(): return false
|
|
||
| func test_wait_until_causes_is_waiting_to_be_true_when_waiting(): | ||
| var node = add_child_autoqfree(Node.new()) | ||
| var is_named_foo = func(): return node.name == 'foo' |
There was a problem hiding this comment.
If this always returns false, it's a little easier to understand the test:
var is_named_foo = func(): return false
| # This test passes, but prints the error log: | ||
| # ERROR: Lambda capture at index 0 was freed. Passed "null" instead | ||
| # Godot issue: https://github.com/godotengine/godot/issues/85947 | ||
| func test_wait_until_is_compatible_with_checking_if_an_object_is_freed(): |
There was a problem hiding this comment.
In addons/gut/utils.gd there is the following is_freed method:
func is_freed(obj):
var wr = weakref(obj)
return !(wr.get_ref() and str(obj) != '<Freed Object>')
If I change var is_freed to:
var is_freed = _utils.is_freed.bind(node)
the test passes without error.
I'm not sure if this should be another test case or not. Maybe using is_freed should be documented somewhere.
EDIT: I just searched the codebase and utils.is_freed is not used anywhere, but utils.is_not_freed is used in one spot. is_instance_valid is used in a lot of places. Maybe is_freed is a hold over from the godot 4 conversion and should be removed. Do you have any thoughts?
|
|
||
| assert_between(counter.time, 0, .2) |
There was a problem hiding this comment.
Change 0 to 0.0 to get rid of a GUT int/float warning.
First step for issue: #585
Replaces #577. Closing it.