Skip to content
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

Remove need for delays when testing neovim jobcontrol #235

Open
okkays opened this issue Aug 6, 2020 · 2 comments
Open

Remove need for delays when testing neovim jobcontrol #235

okkays opened this issue Aug 6, 2020 · 2 comments

Comments

@okkays
Copy link
Contributor

okkays commented Aug 6, 2020

In #234, (0.2s) delay controls were added to CallAsync tests in system-job.vroom. Figure out why that extra delay is needed, fix the cause, and remove the delays.

Some notes:

  • The commands don't seem to need the extra delay in practice (plugins depending on the jobcontrol implementation use it and work fine)
  • Per debugging detailed in this comment, with no delay, it appears that maktaba#syscall#neovim#HandleJobExit is receiving the stdout of the call, but shell.vroomfaker isn't reporting that it received it.
dbarnett pushed a commit that referenced this issue Aug 6, 2020
Support Syscall.CallAsync in neovim using neovim jobs.

Rename system-vimjob.vroom to system-job.vroom and have it
cover both vim and neovim implementations (adding delays as
a workaround for async timing quirks in neovim, #235).
@dbarnett
Copy link
Contributor

dbarnett commented Nov 1, 2020

I did a little more digging on this. Still no real answers, but a few observations:

  • Might involve buffered I/O since IIRC the output files written by vroom.shellfaker are the way it "receives" system calls. I tried adding some flush() calls into the shellfaker but didn't see any improvement.
  • Might help to compare strace output with/without the extra delay (strace -o strace.out vroom --neovim vroom/system-job.vroom). I saw a few differences but got a little stuck making sense of them.

I guess generally this shouldn't be a concern outside of maktaba tests since tests should override and disable async execution anyway (except for tests like this specifically trying to cover async execution behavior). Just frustrating to not understand why the workaround delays are needed.

@dbarnett
Copy link
Contributor

FYI I've been running into other timing issues in neovim mode, this time just executing a foreground shell command with :!, when running in Travis CI anyway (see google/vroom#2).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants