From 3c4b87bfec5d06fcde67aaee961e1b9ca637cae7 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Fri, 20 Nov 2020 13:32:46 +0200 Subject: [PATCH] checker: add check against `fn test_opt() ?{}` --- vlib/v/checker/checker.v | 21 +++++--- .../test_functions_should_not_return_test.out | 13 +++++ .../test_functions_should_not_return_test.vv | 15 ++++++ vlib/v/tests/run_v_code_from_stdin_test.v | 8 ++- vlib/v/tests/unsafe_test.v | 11 ++-- vlib/v/vcache/vcache_test.v | 50 ++++++++++++++----- vlib/x/websocket/websocket_test.v | 10 ++-- 7 files changed, 98 insertions(+), 30 deletions(-) create mode 100644 vlib/v/checker/tests/test_functions_should_not_return_test.out create mode 100644 vlib/v/checker/tests/test_functions_should_not_return_test.vv diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 1df3a0b4a5..b9497494ad 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -4505,16 +4505,21 @@ fn (mut c Checker) fn_decl(mut node ast.FnDecl) { } } // TODO c.pref.is_vet - if node.language == .v && !node.is_method && node.params.len == 0 && node.return_type == table.void_type_idx && - node.name.after('.').starts_with('test_') && !c.file.path.ends_with('_test.v') { - // simple heuristic - for st in node.stmts { - if st is ast.AssertStmt { - c.warn('tests will not be run because filename does not end with `_test.v`', - node.pos) - break + if node.language == .v && !node.is_method && node.params.len == 0 && node.name.after('.').starts_with('test_') { + if !c.file.path.ends_with('_test.v') { + // simple heuristic + for st in node.stmts { + if st is ast.AssertStmt { + c.warn('tests will not be run, because filename does not end with `_test.v`', + node.pos) + break + } } } + // eprintln('> node.name: $node.name | node.return_type: $node.return_type') + if node.return_type != table.void_type_idx { + c.error('test functions should not return anything', node.pos) + } } c.expected_type = table.void_type c.cur_fn = node diff --git a/vlib/v/checker/tests/test_functions_should_not_return_test.out b/vlib/v/checker/tests/test_functions_should_not_return_test.out new file mode 100644 index 0000000000..4442e75647 --- /dev/null +++ b/vlib/v/checker/tests/test_functions_should_not_return_test.out @@ -0,0 +1,13 @@ +vlib/v/checker/tests/test_functions_should_not_return_test.vv:9:1: error: test functions should not return anything + 7 | + 8 | // should be disallowed: + 9 | fn test_returning_int() int { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 10 | + 11 | } +vlib/v/checker/tests/test_functions_should_not_return_test.vv:14:1: error: test functions should not return anything + 12 | + 13 | // should be disallowed: + 14 | fn test_returning_opt() ? { + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + 15 | } diff --git a/vlib/v/checker/tests/test_functions_should_not_return_test.vv b/vlib/v/checker/tests/test_functions_should_not_return_test.vv new file mode 100644 index 0000000000..3255a262f2 --- /dev/null +++ b/vlib/v/checker/tests/test_functions_should_not_return_test.vv @@ -0,0 +1,15 @@ +// ordinary functions can return whatever they like, +// since they are not called by V's testing system +// in the generated main(): +fn abc() int { + return 1 +} + +// should be disallowed: +fn test_returning_int() int { + +} + +// should be disallowed: +fn test_returning_opt() ? { +} diff --git a/vlib/v/tests/run_v_code_from_stdin_test.v b/vlib/v/tests/run_v_code_from_stdin_test.v index 4e175c612c..aa865c4bd9 100644 --- a/vlib/v/tests/run_v_code_from_stdin_test.v +++ b/vlib/v/tests/run_v_code_from_stdin_test.v @@ -8,7 +8,7 @@ fn test_vexe_is_set() { assert vexe != '' } -fn test_pipe_to_v_run() ? { +fn pipe_to_v_run() ? { cat_cmd := if os.user_os() == 'windows' { 'type' } else { 'cat' } tmp_v_file := os.join_path(os.temp_dir(), 'generated_piped_program.v') os.write_file(tmp_v_file, 'println(1 + 3)\nprintln("hello")\n') ? @@ -21,3 +21,9 @@ fn test_pipe_to_v_run() ? { os.rm(tmp_v_file) assert !os.exists(tmp_v_file) } + +fn test_pipe_to_v_run() { + pipe_to_v_run() or { + panic(err) + } +} diff --git a/vlib/v/tests/unsafe_test.v b/vlib/v/tests/unsafe_test.v index 601fc45c0f..f79cd06f73 100644 --- a/vlib/v/tests/unsafe_test.v +++ b/vlib/v/tests/unsafe_test.v @@ -23,9 +23,7 @@ fn test_double_ptr() { (*p) = &j assert x == &j } - - ///////// - + // /////// mut x := &int(0) unsafe { mut p := &x @@ -60,7 +58,7 @@ fn test_if_expr_unsafe() { assert *p == 4 } -fn test_unsafe_if_stmt() int { +fn unsafe_if_stmt() int { i := 4 unsafe { if true { @@ -70,6 +68,11 @@ fn test_unsafe_if_stmt() int { return i } +fn test_unsafe_if_stmt() { + x := unsafe_if_stmt() + assert x == 4 +} + fn test_map_address_index() { mut m := { 'one': 1 diff --git a/vlib/v/vcache/vcache_test.v b/vlib/v/vcache/vcache_test.v index 92b9877a55..c0a202bfba 100644 --- a/vlib/v/vcache/vcache_test.v +++ b/vlib/v/vcache/vcache_test.v @@ -14,7 +14,7 @@ fn check_cache_entry_fpath_invariants(x string, extension string) { assert a[1][0..2] == a[0] } -fn testsuite_begin() ? { +fn testsuite_begin() { os.setenv('VCACHE', vcache_folder, true) // eprintln('testsuite_begin, vcache_folder = $vcache_folder') os.rmdir_all(vcache_folder) @@ -22,41 +22,62 @@ fn testsuite_begin() ? { assert os.is_dir(vcache_folder) } -fn test_save_and_load() ? { +fn test_save_and_load() { mut cm := vcache.new_cache_manager([]) - x := cm.save('.txt', 'first/cache/entry', 'hello') ? + x := cm.save('.txt', 'first/cache/entry', 'hello') or { + assert false + '' + } check_cache_entry_fpath_invariants(x, '.txt') } -fn test_different_options_should_produce_different_cache_entries_for_same_key_and_content() ? { +fn test_different_options_should_produce_different_cache_entries_for_same_key_and_content() { mut cm1 := vcache.new_cache_manager([]) mut cm2 := vcache.new_cache_manager(['-cc tcc']) mut cm3 := vcache.new_cache_manager(['-cc gcc']) - x := cm1.save('.txt', 'first/cache/entry', 'hello') ? - y := cm2.save('.txt', 'first/cache/entry', 'hello') ? - z := cm3.save('.txt', 'first/cache/entry', 'hello') ? + x := cm1.save('.txt', 'first/cache/entry', 'hello') or { + assert false + '' + } + y := cm2.save('.txt', 'first/cache/entry', 'hello') or { + assert false + '' + } + z := cm3.save('.txt', 'first/cache/entry', 'hello') or { + assert false + '' + } check_cache_entry_fpath_invariants(x, '.txt') check_cache_entry_fpath_invariants(y, '.txt') check_cache_entry_fpath_invariants(z, '.txt') } -fn test_exists() ? { +fn test_exists() { mut cm := vcache.new_cache_manager([]) cm.exists('.o', 'abc') or { assert true } // - x := cm.save('.x', 'abc', '') ? + x := cm.save('.x', 'abc', '') or { + assert false + '' + } cm.exists('.o', 'abc') or { assert true } // - y := cm.save('.o', 'zbc', '') ? + y := cm.save('.o', 'zbc', '') or { + assert false + '' + } cm.exists('.o', 'abc') or { assert true } // - z := cm.save('.o', 'abc', '') ? + z := cm.save('.o', 'abc', '') or { + assert false + '' + } cm.exists('.o', 'abc') or { assert false } @@ -69,11 +90,14 @@ fn test_exists() ? { assert y != z } -fn test_readme_exists_and_is_readable() ? { +fn test_readme_exists_and_is_readable() { vcache.new_cache_manager([]) freadme := os.join_path(vcache_folder, 'README.md') assert os.is_file(freadme) - x := os.read_file(freadme) ? + x := os.read_file(freadme) or { + assert false + '' + } assert x.len > 0 assert x.starts_with('This folder contains cached build artifacts') } diff --git a/vlib/x/websocket/websocket_test.v b/vlib/x/websocket/websocket_test.v index 050da93f75..74e7659881 100644 --- a/vlib/x/websocket/websocket_test.v +++ b/vlib/x/websocket/websocket_test.v @@ -2,10 +2,12 @@ import x.websocket import time // Tests with external ws & wss servers -fn test_ws() ? { +fn test_ws() { go start_server() time.sleep_ms(100) - ws_test('ws://localhost:30000')? + ws_test('ws://localhost:30000') or { + assert false + } } fn start_server() ? { @@ -20,7 +22,7 @@ fn start_server() ? { return false } return true - })? + }) ? s.on_message(fn (mut ws websocket.Client, msg &websocket.Message) ? { // payload := if msg.payload.len == 0 { '' } else { string(msg.payload, msg.payload.len) } // println('server client ($ws.id) got message: opcode: $msg.opcode, payload: $payload') @@ -38,7 +40,7 @@ fn start_server() ? { fn ws_test(uri string) ? { eprintln('connecting to $uri ...') - mut ws := websocket.new_client(uri)? + mut ws := websocket.new_client(uri) ? ws.on_open(fn (mut ws websocket.Client) ? { println('open!') ws.pong()