From 549fc9548deaa737ff101806b60cedd7d4970c28 Mon Sep 17 00:00:00 2001 From: Thiago de Arruda Date: Wed, 1 Oct 2014 09:05:28 -0300 Subject: [PATCH 1/6] test: Move 'test/legacy' to 'test/functional' Busted can only discover tests from a single directory. In order to allow tests under 'legacy' to run as a functional test, it needed to be moved to 'test/functional'. --- cmake/RunTests.cmake | 2 +- test/{ => functional}/legacy/002_filename_recognition_spec.lua | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test/{ => functional}/legacy/002_filename_recognition_spec.lua (100%) diff --git a/cmake/RunTests.cmake b/cmake/RunTests.cmake index dc02ce5400..b89957bb28 100644 --- a/cmake/RunTests.cmake +++ b/cmake/RunTests.cmake @@ -7,7 +7,7 @@ endif() if(TEST_TYPE STREQUAL "functional") execute_process( COMMAND python ${BUSTED_PRG} ${BUSTED_REAL_PRG} -v -o - ${BUSTED_OUTPUT_TYPE} --lpath=${BUILD_DIR}/?.lua ${TEST_DIR}/legacy + ${BUSTED_OUTPUT_TYPE} --lpath=${BUILD_DIR}/?.lua ${TEST_DIR}/functional WORKING_DIRECTORY ${WORKING_DIR} RESULT_VARIABLE res) else() diff --git a/test/legacy/002_filename_recognition_spec.lua b/test/functional/legacy/002_filename_recognition_spec.lua similarity index 100% rename from test/legacy/002_filename_recognition_spec.lua rename to test/functional/legacy/002_filename_recognition_spec.lua From f6a008a1824938038bb0801881302ea5b42e1087 Mon Sep 17 00:00:00 2001 From: Thiago de Arruda Date: Wed, 1 Oct 2014 09:31:57 -0300 Subject: [PATCH 2/6] test: Add 'eval' functional helper The eval helper transforms vimL expressions into lua tables, it's useful for verifying function output. --- scripts/run-functional-tests.py | 14 +++++++++++++- test/functional/helpers.lua | 22 ++++++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/scripts/run-functional-tests.py b/scripts/run-functional-tests.py index 1b8fb2ddef..3e931b248c 100644 --- a/scripts/run-functional-tests.py +++ b/scripts/run-functional-tests.py @@ -4,7 +4,7 @@ import os import sys import textwrap -from lupa import LuaRuntime +from lupa import LuaRuntime, as_attrgetter from neovim import Nvim, spawn_session @@ -33,6 +33,14 @@ function(d) end ''') +def to_table(obj): + if type(obj) in [tuple, list]: + return list_to_table(list(to_table(e) for e in obj)) + if type(obj) is dict: + return dict_to_table(as_attrgetter( + dict((k, to_table(v)) for k, v in obj.items()))) + return obj + nvim_prog = os.environ.get('NVIM_PROG', 'build/bin/nvim') nvim_argv = [nvim_prog, '-u', 'NONE', '--embed'] @@ -51,6 +59,9 @@ nvim = Nvim.from_session(session) def nvim_command(cmd): nvim.command(cmd) +def nvim_eval(expr): + return to_table(nvim.eval(expr)) + def nvim_feed(input, mode=''): nvim.feedkeys(input) @@ -63,6 +74,7 @@ def nvim_replace_termcodes(input, *opts): expose = [ nvim_command, + nvim_eval, nvim_feed, nvim_replace_termcodes, buffer_slice, diff --git a/test/functional/helpers.lua b/test/functional/helpers.lua index 2b9ddfbe3c..671e34e592 100644 --- a/test/functional/helpers.lua +++ b/test/functional/helpers.lua @@ -31,9 +31,24 @@ local function execute(...) end end +local function eval(expr) + local status, result = pcall(function() return nvim_eval(expr) end) + if not status then + error('Failed to evaluate expression "' .. expr .. '"') + end + return result +end + +local function eq(expected, actual) + return assert.are.same(expected, actual) +end + +local function neq(expected, actual) + return assert.are_not.same(expected, actual) +end + local function expect(contents, first, last, buffer_index) - return assert.are.same(dedent(contents), - buffer_slice(first, last, buffer_idx)) + return eq(dedent(contents), buffer_slice(first, last, buffer_idx)) end rawfeed([[:function BeforeEachTest() @@ -80,5 +95,8 @@ return { insert = insert, feed = feed, execute = execute, + eval = eval, + eq = eq, + neq = neq, expect = expect } From 35d7815eb2c89bd88c94e32b2e6afecc906d7e7d Mon Sep 17 00:00:00 2001 From: Thiago de Arruda Date: Wed, 1 Oct 2014 09:32:56 -0300 Subject: [PATCH 3/6] test: Add some specs for the viml function `system()` These new specs replace src/nvim/testdir/test_system --- src/nvim/testdir/Makefile | 3 +- src/nvim/testdir/test_system.in | Bin 137 -> 0 bytes src/nvim/testdir/test_system.ok | 3 - test/functional/shell/viml_system_spec.lua | 125 +++++++++++++++++++++ 4 files changed, 126 insertions(+), 5 deletions(-) delete mode 100644 src/nvim/testdir/test_system.in delete mode 100644 src/nvim/testdir/test_system.ok create mode 100644 test/functional/shell/viml_system_spec.lua diff --git a/src/nvim/testdir/Makefile b/src/nvim/testdir/Makefile index 26bf35aa94..9f04f880b5 100644 --- a/src/nvim/testdir/Makefile +++ b/src/nvim/testdir/Makefile @@ -35,8 +35,7 @@ SCRIPTS := test_autoformat_join.out \ test_listlbr.out test_listlbr_utf8.out \ test_changelist.out \ test_breakindent.out \ - test_insertcount.out \ - test_systen.in + test_insertcount.out SCRIPTS_GUI := test16.out diff --git a/src/nvim/testdir/test_system.in b/src/nvim/testdir/test_system.in deleted file mode 100644 index 420465ce26da69eb49dc974fd564038c900c5e61..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 137 zcmd-I4si?$32_Y$;o`DNRfs4_EiU28OiW@(PLbxaDhG*yg!J-DOSr653Q9{9iYtpt zQgb!blM_oI%GEVdWO6c#OVDJ?3l+Jnk`r@s6jD-iQcF@b)WP-uHFCN7xj-xj0A@8R ALI3~& diff --git a/src/nvim/testdir/test_system.ok b/src/nvim/testdir/test_system.ok deleted file mode 100644 index aa60536c3b..0000000000 --- a/src/nvim/testdir/test_system.ok +++ /dev/null @@ -1,3 +0,0 @@ - -abcd -['abcd'] diff --git a/test/functional/shell/viml_system_spec.lua b/test/functional/shell/viml_system_spec.lua new file mode 100644 index 0000000000..effabe715c --- /dev/null +++ b/test/functional/shell/viml_system_spec.lua @@ -0,0 +1,125 @@ +-- Specs for +-- - `system()` +-- - `systemlist()` + +local helpers = require('test.functional.helpers') +local eq, clear, eval, feed = + helpers.eq, helpers.clear, helpers.eval, helpers.feed + + +local function create_file_with_nuls(name) + return function() + feed('ipart1000part2000part3:w '..name..'') + end +end + +local function delete_file(name) + return function() + eval("delete('"..name.."')") + end +end + + +describe('system()', function() + before_each(clear) + + describe('passing no input', function() + it('returns the program output', function() + eq("echoed", eval('system("echo -n echoed")')) + end) + end) + + describe('passing input', function() + it('returns the program output', function() + eq("input", eval('system("cat -", "input")')) + end) + end) + + describe('passing number as input', function() + it('stringifies the input', function() + eq('1', eval('system("cat", 1)')) + end) + end) + + describe('with output containing NULs', function() + local fname = 'Xtest' + + setup(create_file_with_nuls(fname)) + teardown(delete_file(fname)) + + it('replaces NULs by SOH characters', function() + eq('part1\001part2\001part3\n', eval('system("cat '..fname..'")')) + end) + end) + + describe('passing list as input', function() + it('joins list items with linefeed characters', function() + eq('line1\nline2\nline3', + eval("system('cat -', ['line1', 'line2', 'line3'])")) + end) + + -- Notice that NULs are converted to SOH when the data is read back. This + -- is inconsistent and is a good reason for the existence of the + -- `systemlist()` function, where input and output map to the same + -- characters(see the following tests with `systemlist()` below) + describe('with linefeed characters inside list items', function() + it('converts linefeed characters to NULs', function() + eq('l1\001p2\nline2\001a\001b\nl3', + eval([[system('cat -', ["l1\np2", "line2\na\nb", 'l3'])]])) + end) + end) + + describe('with leading/trailing whitespace characters on items', function() + it('preserves whitespace, replacing linefeeds by NULs', function() + eq('line \nline2\001\n\001line3', + eval([[system('cat -', ['line ', "line2\n", "\nline3"])]])) + end) + end) + end) +end) + +describe('systemlist()', function() + -- behavior is similar to `system()` but it returns a list instead of a + -- string. + before_each(clear) + + describe('passing string with linefeed characters as input', function() + it('splits the output on linefeed characters', function() + eq({'abc', 'def', 'ghi'}, eval([[systemlist("cat -", "abc\ndef\nghi")]])) + end) + end) + + describe('with output containing NULs', function() + local fname = 'Xtest' + + setup(create_file_with_nuls(fname)) + teardown(delete_file(fname)) + + it('replaces NULs by newline characters', function() + eq({'part1\npart2\npart3'}, eval('systemlist("cat '..fname..'")')) + end) + end) + + describe('passing list as input', function() + it('joins list items with linefeed characters', function() + eq({'line1', 'line2', 'line3'}, + eval("systemlist('cat -', ['line1', 'line2', 'line3'])")) + end) + + -- Unlike `system()` which uses SOH to represent NULs, with `systemlist()` + -- input and ouput are the same + describe('with linefeed characters inside list items', function() + it('converts linefeed characters to NULs', function() + eq({'l1\np2', 'line2\na\nb', 'l3'}, + eval([[systemlist('cat -', ["l1\np2", "line2\na\nb", 'l3'])]])) + end) + end) + + describe('with leading/trailing whitespace characters on items', function() + it('preserves whitespace, replacing linefeeds by NULs', function() + eq({'line ', 'line2\n', '\nline3'}, + eval([[systemlist('cat -', ['line ', "line2\n", "\nline3"])]])) + end) + end) + end) +end) From 93a45ccc25f2e3f985b631b70e321414b4209d53 Mon Sep 17 00:00:00 2001 From: Thiago de Arruda Date: Wed, 1 Oct 2014 19:30:13 -0300 Subject: [PATCH 4/6] travis: Fix clang-asan to always display memory errors Always check the logs in case of test failures(which would happen when ASAN finds an error since it will abort Nvim). Also run the 'oldtest' target from the gcc-32.sh script --- .ci/clang-asan.sh | 8 ++++++-- .ci/gcc-32.sh | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.ci/clang-asan.sh b/.ci/clang-asan.sh index 5019466fcf..0dff61f297 100644 --- a/.ci/clang-asan.sh +++ b/.ci/clang-asan.sh @@ -26,15 +26,19 @@ export UBSAN_OPTIONS="log_path=$tmpdir/ubsan" # not sure if this works install_dir="$(pwd)/dist" $MAKE_CMD cmake CMAKE_EXTRA_FLAGS="-DTRAVIS_CI_BUILD=ON -DCMAKE_INSTALL_PREFIX=$install_dir -DUSE_GCOV=ON" -$MAKE_CMD test +if ! $MAKE_CMD test; then + asan_check "$tmpdir" + exit 1 +fi asan_check "$tmpdir" + if ! $MAKE_CMD oldtest; then reset asan_check "$tmpdir" exit 1 fi - asan_check "$tmpdir" + coveralls --encoding iso-8859-1 || echo 'coveralls upload failed.' $MAKE_CMD install diff --git a/.ci/gcc-32.sh b/.ci/gcc-32.sh index c0e9dcd839..ec51cfab69 100644 --- a/.ci/gcc-32.sh +++ b/.ci/gcc-32.sh @@ -28,3 +28,4 @@ CMAKE_EXTRA_FLAGS="-DTRAVIS_CI_BUILD=ON \ $MAKE_CMD CMAKE_EXTRA_FLAGS="${CMAKE_EXTRA_FLAGS}" unittest $MAKE_CMD test +$MAKE_CMD oldtest From ba1026c2c7a39b92e5c50bee653cb7bca3d1f759 Mon Sep 17 00:00:00 2001 From: Thiago de Arruda Date: Wed, 1 Oct 2014 19:43:00 -0300 Subject: [PATCH 5/6] eval: Fix `save_tv_as_string` to handle non-string types --- src/nvim/eval.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nvim/eval.c b/src/nvim/eval.c index 114a6f8a59..b0aa5a4162 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -15148,8 +15148,6 @@ static char_u *save_tv_as_string(typval_T *tv, ssize_t *len) ret = vim_strsave(ret); } else { ret = NULL; - } - if (tv->v_type != VAR_STRING) { *len = -1; } return ret; From 45525853d3521e73512f68bb390aae0e385ef66c Mon Sep 17 00:00:00 2001 From: Thiago de Arruda Date: Wed, 1 Oct 2014 20:19:10 -0300 Subject: [PATCH 6/6] wstream/shell: Fix memory errors caused by os_system The os_system function uses a write callback to close the input stream when the write completes, but this causes a memory error because the callback is invoked right before the stream is freed by the caller. This fixes the problem by removing the callback set by os_system. Instead, it calls job_close_in immediately after writing(the stream will only close after the write completes). The 'pending' parameter was also removed from the 'write_cb' as it should be hidden by the wstream module. While the `wstream_set_write_cb` and `job_write_cb` are no longer used, they will remain in the codebase for future use. --- src/nvim/os/shell.c | 19 ++----------------- src/nvim/os/wstream.c | 5 ++--- src/nvim/os/wstream_defs.h | 2 -- 3 files changed, 4 insertions(+), 22 deletions(-) diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c index 398d94b606..912dc95aca 100644 --- a/src/nvim/os/shell.c +++ b/src/nvim/os/shell.c @@ -293,19 +293,15 @@ int os_system(const char *cmd, if (input) { WBuffer *input_buffer = wstream_new_buffer((char *) input, len, 1, NULL); - // we want to be notified when the write completes - job_write_cb(job, system_write_cb); - if (!job_write(job, input_buffer)) { // couldn't write, stop the job and tell the user about it job_stop(job); return -1; } - } else { - // close the input stream, let the process know that no input is coming - job_close_in(job); } + // close the input stream, let the process know that no more input is coming + job_close_in(job); int status = job_wait(job, -1); // prepare the out parameters if requested @@ -353,17 +349,6 @@ static void system_data_cb(RStream *rstream, void *data, bool eof) buf->len += nread; } -static void system_write_cb(WStream *wstream, - void *data, - size_t pending, - int status) -{ - if (pending == 0) { - Job *job = data; - job_close_in(job); - } -} - /// Parses a command string into a sequence of words, taking quotes into /// consideration. /// diff --git a/src/nvim/os/wstream.c b/src/nvim/os/wstream.c index 00a53d1628..eb7de02a2f 100644 --- a/src/nvim/os/wstream.c +++ b/src/nvim/os/wstream.c @@ -208,15 +208,14 @@ static void write_cb(uv_write_t *req, int status) release_wbuffer(data->buffer); - data->wstream->pending_reqs--; - if (data->wstream->cb) { data->wstream->cb(data->wstream, data->wstream->data, - data->wstream->pending_reqs, status); } + data->wstream->pending_reqs--; + if (data->wstream->freed && data->wstream->pending_reqs == 0) { // Last pending write, free the wstream; free(data->wstream); diff --git a/src/nvim/os/wstream_defs.h b/src/nvim/os/wstream_defs.h index e42481f283..cfa0bf0b60 100644 --- a/src/nvim/os/wstream_defs.h +++ b/src/nvim/os/wstream_defs.h @@ -10,11 +10,9 @@ typedef void (*wbuffer_data_finalizer)(void *data); /// /// @param wstream The `WStream` instance /// @param data User-defined data -/// @param pending The number of write requests that are still pending /// @param status 0 on success, anything else indicates failure typedef void (*wstream_cb)(WStream *wstream, void *data, - size_t pending, int status); #endif // NVIM_OS_WSTREAM_DEFS_H