From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 130962AFB1 for ; Tue, 9 Apr 2019 07:47:21 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eKBEbqyCqZcP for ; Tue, 9 Apr 2019 07:47:20 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id BCF492AFA8 for ; Tue, 9 Apr 2019 07:47:20 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 3/5] test: speed up swim big cluster failure detection References: <13b859581390bf1f202096fbaa2405204fcbb4dc.1554465150.git.v.shpilevoy@tarantool.org> <20190409084303.GA16539@chai> From: Vladislav Shpilevoy Message-ID: <375d0665-e7d0-5f73-9e48-4f432fc86653@tarantool.org> Date: Tue, 9 Apr 2019 14:47:18 +0300 MIME-Version: 1.0 In-Reply-To: <20190409084303.GA16539@chai> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, Konstantin Osipov On 09/04/2019 11:43, Konstantin Osipov wrote: > * Vladislav Shpilevoy [19/04/05 16:12]: > > The more I look at the tests the more confusing the test function > names become. > > The namespace of test functions is clearly clashing with the main > swim namespace, which makes the tests hard to read and follow: > > it's unclear which function belongs to the tests harness and which > to the swim itself. Please come up with a harness api prefix. Short answer: 'swim_cluster_' is the harness API prefix. There are only two harness' public functions not prefixed: swim_run_for() and swim_error_check_match(), but they are just utilities, and not related to a concrete set of swim instances (cluster). I've fixed these two functions to be prefixed with 'swim_test_'. Long answer below. > > Please feel free to do it in a subsequent patch. I do not understand, how is it supposed to help, because it is not necessary to know which function are public, and which are not, to understand what happens in the test. My logic is that a test should be easy to read like a text, and to understand the scenario. And for that we usually never prefix our test functions with special prefixes. Even in unit tests. You still continue to create new rules so as to forget about them afterwards, and create contradictory ones or just break olds. And you never document any of your ideas. Below you can find some examples of not prefixing test functions. Some of them are simple, some of them (just like swim) are a part of a harness. Examples: - unit/bitset_iterator.c, test functions, not prefixed: * bitsets_create * bitsets_destroy * nums_fill * nums_comparator * nums_sort * nums_shuffle - unit/cbus.c, test functions, not prefixed: * do_nothing * flush_cb * finish_execution * worker_f * worker_start * worker_stop * do_forced_flush * do_some_event - unit/cbus_stress.c, test functions and structures, not prefixed: * struct conn * struct thread * thread_create, thread_name, thread_func, thread_destroy, thread_connect, thread_disconnect, thread_connect_random, thread_disconnect_random, thread_send, thread_send_random * struct thread_msg * thread_msg_received_cb By the way, that test was authored by Vova and *committed by you*. - unit/column_mask.c, test functions and structures, not prefixed: * struct tuple_op_template * struct tuple_update_template * struct tuple_template * tuple_new_raw * tuple_new_update * tuple_update_alloc_f * check_update_result That test was authored by me and *committed by you*. - unit/csv.c, test functions and structures, not prefixed: * print_endl * print_field * buf_endl * buf_field * struct counter * line_counter * fieldsizes_counter * csv_out - unit/fiber.c, test functions, not prefixed: * noop_f * cancel_f * exception_f * no_exception_f * cancel_dead_f * stack_expand Here the names were invented or patched *by you*. - unit/histogram.c, test functions, not prefixed: * int64_cmp * int64_sort * gen_buckets * gen_rand_data * gen_rand_value unit/say.c, test functions and structures, not prefixed: * parse_logger_type * parse_syslog_opts * format_func_custom * struct create_log * dummy_log These parts of the test were authored by Ilya and *committed by you*. My favorite part - Vinyl. It is relatively new, and was almost completely reviewed and committed by you. unit/vy_iterators_helper.c/.h, test functions and structures, not prefixed: * vy_new_simple_stmt * vy_mem_insert_template * vy_cache_insert_templates_chain * vy_cache_on_write_template * init_read_views_list * vy_stmt_are_same * struct vy_stmt_template unit/vy_log.stub.c, test functions, not prefixed: * vy_log_next_id * vy_log_tx_begin * vy_log_tx_commit * vy_log_write * vy_recovery_lsm_by_index_id unit/vy_point_lookup.c: write_run(). I skipped most of the small test files, and decided not to scan non-unit tests. But apparently in non-unit tests we never use prefixes as well. And evidently this lack of useless padding-out-the-code prefixes does not aggravate readability of the tests. What is more, even now all non-test functions can be identified by the first 'struct swim *' and 'struct swim_member *' parameters. Test functions are prefixed with 'swim_cluster_' prefix. The only methods, not prefixed with 'swim_cluster_' here were 'swim_run_for' and 'swim_error_check_match'. I renamed these two methods with a prefix 'swim_test_', and changed 'swim_start/finish_test' macro to 'swim_test_start/finish' to be consistent. Now *all* public test harness methods are either prefixed with 'swim_cluster_' or with 'swim_test_'. I will not send it as a follow-up patch in order to keep git history as clean as possible. Only old tests I will fix separately, before the main patch.