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

Tests which access the X selections hang if something is asserting ownership #4900

Closed
jamessan opened this issue Jun 9, 2016 · 11 comments · Fixed by #10956
Closed

Tests which access the X selections hang if something is asserting ownership #4900

jamessan opened this issue Jun 9, 2016 · 11 comments · Fixed by #10956
Labels
job-control OS processes, spawn test
Milestone

Comments

@jamessan
Copy link
Member

jamessan commented Jun 9, 2016

  • nvim --version: HEAD
  • Vim (version: NA) behaves differently?
  • Operating system/version: Debian sid

Actual behaviour

If a program is currently asserting ownership of an X selection used by a test, the test will hang due to xsel/xclip not exiting.

Expected behaviour

The test completes successfully, regardless of whether the selection is asserted when the test runs.

Steps to reproduce using nvim -u NORC

  • Double-click on a word in a terminal, thus asserting the PRIMARY selection.
  • make functionaltest TEST_FILE=test/functional/legacy/eval_spec.lua

I ran into this while investigating the QB test failures for #4841, although I'm not entirely sure that it's related since I wouldn't expect any selections to be asserted there.

@bfredl
Copy link
Member

bfredl commented Jun 9, 2016

IMO the tests should not change the user clipboards at all (no "save, change and then restore" does not work in general), unless maybe some env flag that could be set in travis and qb where we have control over it.
One can easily switch to a "fake" private clipboard in lua tests using
execute('let &rtp = "test/functional/fixtures,".&rtp')
right after clear().

@jamessan
Copy link
Member Author

jamessan commented Jun 9, 2016

At least one of those tests (viml_system_spec.lua) is explicitly testing handling of child processes with a specific behavior (exhibited by xclip). Maybe a simple test program that exhibits the same behavior would be a better solution there.

@justinmk
Copy link
Member

Maybe a simple test program that exhibits the same behavior would be a better solution there.

Here is a snippet: #4798 (comment)

@jamessan
Copy link
Member Author

I'm not sure the sleep is as relevant for this scenario is just having the child process not explicitly close its stdout/stderr handles.

@justinmk
Copy link
Member

Can this be closed?

@jamessan
Copy link
Member Author

I don't think so. The viml_system_spec test is still using xclip, which may encounter this issue.

Even if that wasn't a problem, I think having a small test program that exhibits the problem is better than relying on the environment having xclip installed, since it ensures we're testing the problem consistently and doesn't mess with the user's environment.

@justinmk
Copy link
Member

Fixed by #4798

@justinmk justinmk added the job-control OS processes, spawn label Jul 16, 2016
@jamessan
Copy link
Member Author

jamessan commented Feb 1, 2017

This shouldn't have been closed. There's still a test that's invoking xclip, which both messes with the user's selection and prevents the tests from exiting.

@jamessan jamessan reopened this Feb 1, 2017
@justinmk
Copy link
Member

justinmk commented Feb 1, 2017

We could wrap that test in a timeout

@jamessan
Copy link
Member Author

jamessan commented Feb 3, 2017

The test itself completes. The test runner doesn't, because xclip is still running. For example, the last test run I did has been sitting here for the past 12 hours:

●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●●◌
2052 successes / 0 failures / 0 errors / 4 pending : 124.10955 seconds

Pending → .../src/github.com/neovim/test/functional/core/job_spec.lua @ 531
jobs will only emit the "exit" event after "stdout" and "stderr"

Pending → ...github.com/neovim/test/functional/provider/ruby_spec.lua @ 20
Cannot find the neovim RubyGem. Try :CheckHealth

Pending → ...thub.com/neovim/test/functional/terminal/cursor_spec.lua @ 63
terminal cursor with number column is positioned correctly when focused

Pending → ...src/github.com/neovim/test/functional/viml/lang_spec.lua @ 11
viml parses `<SID>` with turkish locale
...src/github.com/neovim/test/functional/viml/lang_spec.lua:11: Locale tr_TR.UTF-8 not supported

14126 pts/5    S+     0:00      \_ make functionaltest
14139 pts/5    S+     0:00          \_ ninja -C build functionaltest
14140 pts/5    Z+     0:00              \_ [sh] <defunct>
15557 pts/5    S+     0:00 xclip -i -selection clipboard

Killing xclip lets it clean up.

@justinmk justinmk added the test label Feb 3, 2017
@justinmk justinmk added this to the todo milestone Feb 3, 2017
justinmk added a commit that referenced this issue Aug 3, 2017
This reverts commit eb40b7e.

The change caused this error on QuickBuild:

    INFO  - # test/functional/core/job_spec.lua @ 668: pty process teardown does not prevent/delay exit. #4798 #4900
    INFO  - not ok 321 - pty process teardown does not prevent/delay exit. #4798 #4900
    INFO  - # test/functional/core/job_spec.lua @ 668
    INFO  - # Failure message: ./test/functional/ui/screen.lua:302: Row 1 did not match.
    INFO  - # Expected:
    INFO  - # |* |
    INFO  - # |[Process exited 0] |
    INFO  - # | |
    INFO  - # | |
    INFO  - # | |
    INFO  - # |-- TERMINAL -- |
    INFO  - # Actual:
    INFO  - # |*E575: Error while reading ShaD|
    INFO  - # |a file: mark entry at position|
    INFO  - # | 92 has invalid line number |
    INFO  - # |Press ENTER or type command to|
    INFO  - # | continue |
    INFO  - # |-- TERMINAL -- |
    INFO  - #
    INFO  - # To print the expect() call that would assert the current screen state, use
    INFO  - # screen:snaphot_util(). In case of non-deterministic failures, use
    INFO  - # screen:redraw_debug() to show all intermediate screen states.
    INFO  - # stack traceback:
    INFO  - #     ./test/functional/ui/screen.lua:302: in function 'wait'
    INFO  - #     ./test/functional/ui/screen.lua:216: in function 'expect'
    INFO  - #     test/functional/core/job_spec.lua:677: in function <test/functional/core/job_spec.lua:668>
janlazo added a commit to janlazo/neovim that referenced this issue Oct 26, 2017
This reverts commit d0673ea.

Testing whether unsetting shada caused the following error in Appveyor

[00:11:35] [  ERROR   ] 1 error, listed below:
[00:11:35] [  ERROR   ] C:/projects/neovim/test/functional\core\job_spec.lua @ 661: pty process teardown does not prevent/delay exit. neovim#4798 neovim#4900
[00:11:35] .\test\functional\ui\screen.lua:302: Row 1 did not match.
[00:11:35] Expected:
[00:11:35]   |*                              |
[00:11:35]   |                              |
[00:11:35]   |                              |
[00:11:35]   |                              |
[00:11:35]   |^[Process exited 0]            |
[00:11:35]   |                              |
[00:11:35] Actual:
[00:11:35]   |*dir\api\buffer.c.gcda:Data fil|
[00:11:35]   |e mismatch - some data files m|
[00:11:35]   |ay have been concurrently upda|
[00:11:35]   |t                             |
[00:11:35]   |^[Process exited -1073741819]  |
[00:11:35]   |                              |
[00:11:35]
[00:11:35] To print the expect() call that would assert the current screen state, use
[00:11:35] screen:snapshot_util(). In case of non-deterministic failures, use
[00:11:35] screen:redraw_debug() to show all intermediate screen states.
[00:11:35]
[00:11:35] stack traceback:
[00:11:35] 	.\test\functional\ui\screen.lua:302: in function 'wait'
[00:11:35] 	.\test\functional\ui\screen.lua:216: in function 'expect'
[00:11:35] 	C:/projects/neovim/test/functional\core\job_spec.lua:672: in function <C:/projects/neovim/test/functional\core\job_spec.lua:661>
@blueyed
Copy link
Contributor

blueyed commented Jun 13, 2019

I agree that we should have a specialized program to test the behavior, and not messing with the clipboard data (#4900 (comment)).

I came here, since the fix / workaround here (-loops 1) causes the test(s) to fail with a clipboard manager (copyq) for me:

[  FAILED  ] test/functional/eval/system_spec.lua @ 382: system() with a program that doesn't close stdout will exit properly after passing input
test/functional/eval/system_spec.lua:384: Expected objects to be the same.
Passed in:
(string) 'Error: target STRING not available
'
Expected:
(string) 'clip-data'

stack traceback:
	test/functional/eval/system_spec.lua:384: in function <test/functional/eval/system_spec.lua:382>

[  FAILED  ] test/functional/eval/system_spec.lua @ 566: systemlist() with a program that doesn't close stdout will exit properly after passing input
test/functional/eval/system_spec.lua:569: Expected objects to be the same.
Passed in:
(table) {
 *[1] = 'Error: target STRING not available' }
Expected:
(table) {
 *[1] = 'clip'
  [2] = 'data' }

stack traceback:
	test/functional/eval/system_spec.lua:569: in function <test/functional/eval/system_spec.lua:566>


 2 FAILED TESTS
-- Output to stderr:

CMake Error at …/Vcs/neovim/cmake/RunTests.cmake:56 (message):
  functional tests failed with error: 1


FAILED: CMakeFiles/functionaltest 

blueyed added a commit to blueyed/neovim that referenced this issue Sep 6, 2019
Replaces "xclip" with a dedicated helper program.

Fixes: neovim#4900 (comment)
blueyed added a commit that referenced this issue Sep 12, 2019
* tests: move os_kill to functional helpers

* tests: fix system_spec when run with clipboard manager

Replaces "xclip" with a dedicated helper program.

Fixes: #4900 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
job-control OS processes, spawn test
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants