Skip to content

Fix Async.Promise tests for test stabiligy even with MacVim #713

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 21, 2020
+23 −19
Merged

Fix Async.Promise tests for test stabiligy even with MacVim #713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2f68967
Async.Promise: Fix MacVim at Travis
ujihisa Jan 20, 2020
1f84c67
Async.Promise: Extend timeout duration only for MacVim
ujihisa Jan 20, 2020
1d77b74
Async.Promise: Extend another timeout in the test
ujihisa Jan 20, 2020
4609809
Async.Promise: test Bugfix: check state after wait
ujihisa Jan 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ jobs:
env:
- VIM_VERSION=master
- THEMIS_PROFILE=vim-profile-master.txt
# - name: Vim MacVim latest
# os: osx
# osx_image: xcode9.4
# env:
# - THEMIS_PROFILE=vim-profile-osx.txt
# cache:
# directories:
# - $HOME/Library/Caches/Homebrew/
# - /usr/local/Homebrew/Library/Homebrew/vendor/
# - /usr/local/Homebrew/Library/Taps/
# before_cache:
# - brew cleanup
# - name: Vim Lint only
# os: linux
# env:
# - RUN_LINT=true
- name: Vim MacVim latest
os: osx
osx_image: xcode9.4
env:
- THEMIS_PROFILE=vim-profile-osx.txt
cache:
directories:
- $HOME/Library/Caches/Homebrew/
- /usr/local/Homebrew/Library/Homebrew/vendor/
- /usr/local/Homebrew/Library/Taps/
before_cache:
- brew cleanup
- name: Vim Lint only
os: linux
env:
- RUN_LINT=true

addons:
apt:
Expand Down
10 changes: 7 additions & 3 deletions test/Async/Promise.vimspec
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
function! s:wait_has_key(obj, name) abort
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may

I feel it's better to remove s:wait_has_key_with_timeout and make a single function s:wait_has_key because

  1. Reducing the number of function helps to reduce the number of noise
  2. The error message said s:wait_has_key() so it's a bit confusing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks! How's this? #714

let i = 0
while i < 500
while i < a:timeout_sec * 100
sleep 10m
if has_key(a:obj, a:name)
return
endif
let i += 1
endwhile
throw printf("s:wait_has_key(): After 5000ms, the given object does not have key '%s': %s", a:name, a:obj)
throw printf("s:wait_has_key(): After %ds, the given object does not have key '%s': %s", a:timeout_sec, a:name, a:obj)
endfunction

function! s:resolver(resolve, reject) abort
Expand Down Expand Up @@ -606,10 +610,10 @@ Describe Async.Promise
\ P.resolve(P.resolve(P.resolve(P.resolve(42)))),
\ P.resolve(Wait(10).then({-> 42}))
\ ]
Assert Equals(p._state, FULFILLED)
let l = l:
call p.then({x -> extend(l, {'result' : x})})
call s:wait_has_key(l, 'result')
Assert Equals(p._state, FULFILLED)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📚

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 🎉

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!

Assert Equals(result, 42)
unlet result
endfor
Expand Down