From 8a8c6b93da6829e0297fdc93b76f2a3933ae8fad Mon Sep 17 00:00:00 2001 From: Romain Bardou Date: Thu, 15 Jun 2023 18:53:16 +0200 Subject: [PATCH 1/3] CI: disable tests with flaky tag --- .gitlab/ci/jobs/test/tezt.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab/ci/jobs/test/tezt.yml b/.gitlab/ci/jobs/test/tezt.yml index 7b54fa873ca7..af584c4e00ce 100644 --- a/.gitlab/ci/jobs/test/tezt.yml +++ b/.gitlab/ci/jobs/test/tezt.yml @@ -39,9 +39,9 @@ tezt: - .template__coverage_files - .tezt_template variables: - # Do not tests with the tag 'ci_disable'. See the registration of + # Do not tests with the tag 'ci_disable' and 'flaky'. See the registration of # the individual test for the disabling rationale. - TESTS: "/ci_disable" + TESTS: "/ci_disable /flaky" dependencies: - "build_x86_64-released" - "build_x86_64-exp-dev-extra" -- GitLab From 39096b84a4215612314c07153f718443a4dccb39 Mon Sep 17 00:00:00 2001 From: Romain Bardou Date: Thu, 15 Jun 2023 19:05:41 +0200 Subject: [PATCH 2/3] Tezt: add Tag module --- .gitlab/ci/jobs/test/tezt.yml | 4 +-- tezt/lib_tezos/tag.ml | 28 ++++++++++++++++++ tezt/lib_tezos/tag.mli | 53 +++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 tezt/lib_tezos/tag.ml create mode 100644 tezt/lib_tezos/tag.mli diff --git a/.gitlab/ci/jobs/test/tezt.yml b/.gitlab/ci/jobs/test/tezt.yml index af584c4e00ce..3bd88abf499f 100644 --- a/.gitlab/ci/jobs/test/tezt.yml +++ b/.gitlab/ci/jobs/test/tezt.yml @@ -39,8 +39,8 @@ tezt: - .template__coverage_files - .tezt_template variables: - # Do not tests with the tag 'ci_disable' and 'flaky'. See the registration of - # the individual test for the disabling rationale. + # Do not tests with the tag 'ci_disable' and 'flaky'. + # See tezt/lib_tezos/tag.mli for more information. TESTS: "/ci_disable /flaky" dependencies: - "build_x86_64-released" diff --git a/tezt/lib_tezos/tag.ml b/tezt/lib_tezos/tag.ml new file mode 100644 index 000000000000..072e5ebdd5f7 --- /dev/null +++ b/tezt/lib_tezos/tag.ml @@ -0,0 +1,28 @@ +(*****************************************************************************) +(* *) +(* Open Source License *) +(* Copyright (c) 2023 Nomadic Labs *) +(* *) +(* Permission is hereby granted, free of charge, to any person obtaining a *) +(* copy of this software and associated documentation files (the "Software"),*) +(* to deal in the Software without restriction, including without limitation *) +(* the rights to use, copy, modify, merge, publish, distribute, sublicense, *) +(* and/or sell copies of the Software, and to permit persons to whom the *) +(* Software is furnished to do so, subject to the following conditions: *) +(* *) +(* The above copyright notice and this permission notice shall be included *) +(* in all copies or substantial portions of the Software. *) +(* *) +(* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR*) +(* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, *) +(* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL *) +(* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER*) +(* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING *) +(* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER *) +(* DEALINGS IN THE SOFTWARE. *) +(* *) +(*****************************************************************************) + +let ci_disable = "ci_disable" + +let flaky = "flaky" diff --git a/tezt/lib_tezos/tag.mli b/tezt/lib_tezos/tag.mli new file mode 100644 index 000000000000..63ca18ab8996 --- /dev/null +++ b/tezt/lib_tezos/tag.mli @@ -0,0 +1,53 @@ +(*****************************************************************************) +(* *) +(* Open Source License *) +(* Copyright (c) 2023 Nomadic Labs *) +(* *) +(* Permission is hereby granted, free of charge, to any person obtaining a *) +(* copy of this software and associated documentation files (the "Software"),*) +(* to deal in the Software without restriction, including without limitation *) +(* the rights to use, copy, modify, merge, publish, distribute, sublicense, *) +(* and/or sell copies of the Software, and to permit persons to whom the *) +(* Software is furnished to do so, subject to the following conditions: *) +(* *) +(* The above copyright notice and this permission notice shall be included *) +(* in all copies or substantial portions of the Software. *) +(* *) +(* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR*) +(* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, *) +(* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL *) +(* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER*) +(* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING *) +(* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER *) +(* DEALINGS IN THE SOFTWARE. *) +(* *) +(*****************************************************************************) + +(** Test tags with specific semantics. *) + +(** The following values are string constants that can be added to [~tags] + in [Test.register]. They have specific semantics that is documented here. + By using [Tag.x] instead of ["x"] in [~tags], you allow developers + to jump to the definition of [Tag.x] and thus see the documentation of the tag. *) + +(** ["flaky"]: the test is flaky. + + This disables the test in the CI just like {!ci_disable}. + Contrary to {!ci_disable} however, this also gives the reason why the test + is disabled: the test is flaky. + + Tip: you can list tests that are declared as flaky with [tezt --list flaky]. + You can check if they are still flaky with something like + [tezt flaky --loop-count 100]. + + Since this tag already conveys information about why the test is disabled, + and since it is easy to get the list of tests with this tag, + it is not necessarily needed to create an issue for each flaky test. *) +val flaky : string + +(** ["ci_disable"]: disable test in the CI. + + The test is not to be run in the CI. + You must provide a comment to explain why the test is disabled. + For flaky tests, {!flaky} should be preferred to [ci_disable]. *) +val ci_disable : string -- GitLab From c0184727f5d85804e8d0cce540348ce1d02488a6 Mon Sep 17 00:00:00 2001 From: Romain Bardou Date: Thu, 15 Jun 2023 19:10:13 +0200 Subject: [PATCH 3/3] Tezt: use Tag module and unify ci_disable{,d} --- .gitlab/ci/jobs/test/tezt.yml | 4 ++-- tezt/lib_tezos/tag.ml | 2 +- tezt/lib_tezos/tag.mli | 13 +++++++------ tezt/long_tests/block_validation.ml | 6 +++--- tezt/long_tests/script_cache.ml | 19 +++++++++++-------- tezt/tests/external_validation.ml | 6 ++---- tezt/tests/synchronisation_heuristic.ml | 21 ++++++++++++++------- 7 files changed, 40 insertions(+), 31 deletions(-) diff --git a/.gitlab/ci/jobs/test/tezt.yml b/.gitlab/ci/jobs/test/tezt.yml index 3bd88abf499f..12a245256398 100644 --- a/.gitlab/ci/jobs/test/tezt.yml +++ b/.gitlab/ci/jobs/test/tezt.yml @@ -39,9 +39,9 @@ tezt: - .template__coverage_files - .tezt_template variables: - # Do not tests with the tag 'ci_disable' and 'flaky'. + # Do not tests with the tag 'ci_disabled' and 'flaky'. # See tezt/lib_tezos/tag.mli for more information. - TESTS: "/ci_disable /flaky" + TESTS: "/ci_disabled /flaky" dependencies: - "build_x86_64-released" - "build_x86_64-exp-dev-extra" diff --git a/tezt/lib_tezos/tag.ml b/tezt/lib_tezos/tag.ml index 072e5ebdd5f7..9a2c94815bc5 100644 --- a/tezt/lib_tezos/tag.ml +++ b/tezt/lib_tezos/tag.ml @@ -23,6 +23,6 @@ (* *) (*****************************************************************************) -let ci_disable = "ci_disable" +let ci_disabled = "ci_disabled" let flaky = "flaky" diff --git a/tezt/lib_tezos/tag.mli b/tezt/lib_tezos/tag.mli index 63ca18ab8996..2c9f45fe4f8e 100644 --- a/tezt/lib_tezos/tag.mli +++ b/tezt/lib_tezos/tag.mli @@ -28,12 +28,13 @@ (** The following values are string constants that can be added to [~tags] in [Test.register]. They have specific semantics that is documented here. By using [Tag.x] instead of ["x"] in [~tags], you allow developers - to jump to the definition of [Tag.x] and thus see the documentation of the tag. *) + to jump to the definition of [Tag.x] and thus see the documentation of the tag. + You also avoid mistakes like using ["ci_disable"] instead of ["ci_disabled"]. *) (** ["flaky"]: the test is flaky. - This disables the test in the CI just like {!ci_disable}. - Contrary to {!ci_disable} however, this also gives the reason why the test + This disables the test in the CI just like {!ci_disabled}. + Contrary to {!ci_disabled} however, this also gives the reason why the test is disabled: the test is flaky. Tip: you can list tests that are declared as flaky with [tezt --list flaky]. @@ -45,9 +46,9 @@ it is not necessarily needed to create an issue for each flaky test. *) val flaky : string -(** ["ci_disable"]: disable test in the CI. +(** ["ci_disabled"]: disable test in the CI. The test is not to be run in the CI. You must provide a comment to explain why the test is disabled. - For flaky tests, {!flaky} should be preferred to [ci_disable]. *) -val ci_disable : string + For flaky tests, {!flaky} should be preferred to [ci_disabled]. *) +val ci_disabled : string diff --git a/tezt/long_tests/block_validation.ml b/tezt/long_tests/block_validation.ml index f3c0240fa0f4..bdc77a2d0f7d 100644 --- a/tezt/long_tests/block_validation.ml +++ b/tezt/long_tests/block_validation.ml @@ -517,7 +517,7 @@ let register ~executors () = Long_test.register ~__FILE__ ~title:Benchmark.chunk_title - ~tags:["shell"; "validation"; "block"; "chunk"; "ci_disabled"] + ~tags:["shell"; "validation"; "block"; "chunk"; Tag.ci_disabled] ~timeout:(Long_test.Minutes 20) ~executors @@ apply_or_raise datadir @@ -526,7 +526,7 @@ let register ~executors () = Long_test.register ~__FILE__ ~title:Benchmark.specific_title - ~tags:["shell"; "validation"; "block"; "specific"; "ci_disabled"] + ~tags:["shell"; "validation"; "block"; "specific"; Tag.ci_disabled] ~timeout:(Long_test.Minutes 20) ~executors @@ apply_or_raise datadir @@ -538,7 +538,7 @@ let register ~executors () = Long_test.register ~__FILE__ ~title:Benchmark.subparts_title - ~tags:["shell"; "validation"; "block"; "subpart"; "ci_disabled"] + ~tags:["shell"; "validation"; "block"; "subpart"; Tag.ci_disabled] ~timeout:(Long_test.Minutes 20) ~executors @@ apply_or_raise datadir diff --git a/tezt/long_tests/script_cache.ml b/tezt/long_tests/script_cache.ml index 8bfe02efe194..2592a6e9e35a 100644 --- a/tezt/long_tests/script_cache.ml +++ b/tezt/long_tests/script_cache.ml @@ -295,7 +295,10 @@ let check ?(tags = []) label test ~protocol ~executors = *) let check_contract_cache_lowers_gas_consumption ~protocol = - check ~tags:["ci_disabled"] "contract cache lowers gas consumption" ~protocol + check + ~tags:[Tag.ci_disabled] + "contract cache lowers gas consumption" + ~protocol @@ fun () -> let* _, client = init1 ~protocol in let* contract_id = originate_str_id_contract client "" in @@ -320,7 +323,7 @@ let check_contract_cache_lowers_gas_consumption ~protocol = *) let check_full_cache ~protocol = check - ~tags:["ci_disabled"] + ~tags:[Tag.ci_disabled] "contract cache does not go beyond its size limit" ~protocol @@ fun () -> @@ -371,7 +374,7 @@ let check_block_impact_on_cache ~protocol = check "one cannot violate the cache size limit" ~protocol - ~tags:["memory"; "limit"; "ci_disabled"] + ~tags:["memory"; "limit"; Tag.ci_disabled] @@ fun () -> let* node, client = init1 ~protocol in @@ -450,7 +453,7 @@ let check_block_impact_on_cache ~protocol = *) let check_cache_backtracking_during_chain_reorganization ~protocol = check - ~tags:["ci_disabled"] + ~tags:[Tag.ci_disabled] "the cache handles chain reorganizations" ~protocol @@ fun () -> @@ -576,7 +579,7 @@ let check_reloading_efficiency ~protocol body = *) let check_cache_reloading_is_not_too_slow ~protocol = - let tags = ["reload"; "performance"; "ci_disabled"] in + let tags = ["reload"; "performance"; Tag.ci_disabled] in check "a node reloads a full cache sufficiently fast" ~protocol ~tags @@ fun () -> check_reloading_efficiency ~protocol @@ fun client -> @@ -637,7 +640,7 @@ let gas_from_simulation client chain_id contract_id ?blocks_before_activation let check_simulation_takes_cache_into_account ~protocol = check "operation simulation takes cache into account" - ~tags:["simulation"; "ci_disabled"] + ~tags:["simulation"; Tag.ci_disabled] ~protocol @@ fun () -> let* _, client = init1 ~protocol in @@ -682,7 +685,7 @@ let check_simulation_close_to_protocol_user_activation ~executors ~migrate_from upgrades" (Protocol.name migrate_from) (Protocol.name migrate_to)) - ~tags:["cache"; "simulation"; "user"; "upgrade"; "ci_disabled"] + ~tags:["cache"; "simulation"; "user"; "upgrade"; Tag.ci_disabled] ~timeout:(Minutes 2000) ~executors @@ fun () -> @@ -781,7 +784,7 @@ let check_simulation_close_to_protocol_auto_activation ~executors ~migrate_from automatic upgrades" (Protocol.name migrate_from) (Protocol.name migrate_to)) - ~tags:["cache"; "simulation"; "auto"; "upgrade"; "ci_disabled"] + ~tags:["cache"; "simulation"; "auto"; "upgrade"; Tag.ci_disabled] ~timeout:(Minutes 2000) ~executors @@ fun () -> diff --git a/tezt/tests/external_validation.ml b/tezt/tests/external_validation.ml index 99ce2a9b2a60..0fe58a2d201b 100644 --- a/tezt/tests/external_validation.ml +++ b/tezt/tests/external_validation.ml @@ -84,10 +84,8 @@ let test_kill = "external"; "validator"; "kill"; - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/5467 - - This test is flaky and has been disabled. *) - "ci_disable"; + (* FIXME: https://gitlab.com/tezos/tezos/-/issues/5467 *) + Tag.flaky; ] @@ fun protocol -> let node = Node.create [] in diff --git a/tezt/tests/synchronisation_heuristic.ml b/tezt/tests/synchronisation_heuristic.ml index 00cef5533fe7..607d7c892aa1 100644 --- a/tezt/tests/synchronisation_heuristic.ml +++ b/tezt/tests/synchronisation_heuristic.ml @@ -76,7 +76,9 @@ let check_node_synchronization_state = ~__FILE__ ~title:"check synchronization state" ~tags: - ["ci_disable"; "synchronisation_threshold"; "bootstrap"; "node"; "sync"] + [ + Tag.ci_disabled; "synchronisation_threshold"; "bootstrap"; "node"; "sync"; + ] @@ fun protocol -> let* main_node = Node.init ~name:"main_node" [] in let* nodes = @@ -163,7 +165,7 @@ let check_prevalidator_start = ~title:"Check prevalidator start" ~tags: [ - "ci_disable"; + Tag.ci_disabled; "synchronisation_threshold"; "bootstrap"; "node"; @@ -227,7 +229,8 @@ let test_threshold_zero = Protocol.register_test ~__FILE__ ~title:"bootstrap: test threshold zero" - ~tags:["ci_disable"; "synchronisation_threshold"; "bootstrap"; "threshold"] + ~tags: + [Tag.ci_disabled; "synchronisation_threshold"; "bootstrap"; "threshold"] @@ fun protocol -> Log.info "Setup network" ; let* node, client = @@ -299,7 +302,8 @@ let test_threshold_two = Protocol.register_test ~__FILE__ ~title:"bootstrap: test threshold two" - ~tags:["ci_disable"; "synchronisation_threshold"; "bootstrap"; "threshold"] + ~tags: + [Tag.ci_disabled; "synchronisation_threshold"; "bootstrap"; "threshold"] @@ fun protocol -> Log.info "Add a first peer with threshold zero" ; let* node, client = @@ -365,7 +369,8 @@ let test_threshold_stuck = Protocol.register_test ~__FILE__ ~title:"bootstrap: test threshold stuck" - ~tags:["ci_disable"; "synchronisation_threshold"; "bootstrap"; "threshold"] + ~tags: + [Tag.ci_disabled; "synchronisation_threshold"; "bootstrap"; "threshold"] @@ fun protocol -> let sync_latency = 3 in @@ -426,7 +431,8 @@ let test_threshold_split_view = Protocol.register_test ~__FILE__ ~title:"bootstrap: test threshold split view" - ~tags:["ci_disable"; "synchronisation_threshold"; "bootstrap"; "threshold"] + ~tags: + [Tag.ci_disabled; "synchronisation_threshold"; "bootstrap"; "threshold"] @@ fun protocol -> Log.info "Add two peers with threshold zero, and one with threshold 2 and a high \ @@ -492,7 +498,8 @@ let test_many_nodes_bootstrap = Protocol.register_test ~__FILE__ ~title:"bootstrap: many nodes bootstrap" - ~tags:["ci_disable"; "synchronisation_threshold"; "bootstrap"; "threshold"] + ~tags: + [Tag.ci_disabled; "synchronisation_threshold"; "bootstrap"; "threshold"] @@ fun protocol -> let num_nodes = 8 in let running_time = 10.0 in -- GitLab