Fix Async.Promise tests for test stabiligy even with MacVim #713
Conversation
ujihisa
force-pushed
the
fix-macvim
branch
4 times, most recently
from
e79b02e to
8cb704d
Compare
ujihisa
changed the title
|
Reviewed-by: Tsuyoshi CHO (Then wait for lambdalisue's reviewing result or just merge now?) |
|
It depends; in this case it's not user-facing changes since the changes are only within test directory. I think it's safer to wait for other people's opinions too only if the changes impact the users or the architecture of vital.vim itself. |
| let l = l: | ||
| call p.then({x -> extend(l, {'result' : x})}) | ||
| call s:wait_has_key(l, 'result') | ||
| Assert Equals(p._state, FULFILLED) |
There was a problem hiding this comment.
📚
The Promise.resolve() method returns a Promise object that is resolved with a given value. If the value is a promise, that promise is returned; if the value is a thenable (i.e. has a "then" method), the returned promise will "follow" that thenable, adopting its eventual state; otherwise the returned promise will be fulfilled with the value. This function flattens nested layers of promise-like objects (e.g. a promise that resolves to a promise that resolves to something) into a single layer.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/resolve
Thus p._state may not be fulfilled just after resolve() if the value is promise or thenable.
That's why the assert timing has changed.
Nice catch 🎉
| return s:wait_has_key_with_timeout(a:obj, a:name, 5) | ||
| endfunction | ||
|
|
||
| function! s:wait_has_key_with_timeout(obj, name, timeout_sec) abort |
There was a problem hiding this comment.
may
I feel it's better to remove s:wait_has_key_with_timeout and make a single function s:wait_has_key because
- Reducing the number of function helps to reduce the number of noise
- The error message said
s:wait_has_key()so it's a bit confusing
There was a problem hiding this comment.
Makes sense, thanks! How's this? #714
* I needed this to fix a flaky test, but after fixing the root cause instead,
we no longer need to specify the timeout from each test case.
* Not having many helper functions in the test increases the maintainability
* #713 (comment)
* I kept the `timeout` as a hard-coded local var instead of hard-coding into the logic as its original form,
for the ease of debug in the future. The runtime cost is ignorable.
#698