From 8a7028358557a35692653a78a4cf419b409eae14 Mon Sep 17 00:00:00 2001 From: Arvid Jakobsson Date: Wed, 27 Mar 2024 18:13:12 +0100 Subject: [PATCH 1/5] CIAO: refactor, inline function [make_rules] With the unit tests inlined, there is no more need for having this function outside [jobs]. Sorry for the peneloping. --- ci/bin/code_verification.ml | 90 ++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/ci/bin/code_verification.ml b/ci/bin/code_verification.ml index 2b54bed7991b..eb55986dba7e 100644 --- a/ci/bin/code_verification.ml +++ b/ci/bin/code_verification.ml @@ -45,50 +45,6 @@ type manual = | Yes (** Add rule for manual trigger. *) | On_changes of Changeset.t (** Add manual trigger on certain [changes:] *) -(* [make_rules] makes rules for jobs that are: - - automatic in scheduled pipelines; - - conditional in [before_merging] pipelines. - - If a job has non-optional dependencies, then [dependent] must be - set to [true] to ensure that we only run the job in case previous - jobs succeeded (setting [when: on_success]). - - If [label] is set, add rule that selects the job in - [Before_merging] pipelines for merge requests with the given - label. Rules for manual triggers can be configured using - [manual]. - - If [label], [changes] and [manual] are omitted, then rules will - enable the job [On_success] in the [before_merging] - pipeline. This is safe, but prefer specifying a [changes] clause - if possible. *) -let make_rules ?label ?changes ?(manual = No) ?(dependent = false) pipeline_type - = - match pipeline_type with - | Schedule_extended_test -> - (* The scheduled pipeline always runs all jobs unconditionally - -- unless they are dependent on a previous, non-trigger job, in the - pipeline. *) - [job_rule ~when_:(if dependent then On_success else Always) ()] - | Before_merging -> ( - (* MR labels can be used to force tests to run. *) - (match label with - | Some label -> - [job_rule ~if_:Rules.(has_mr_label label) ~when_:On_success ()] - | None -> []) - (* Modifying some files can force tests to run. *) - @ (match changes with - | None -> [] - | Some changes -> - [job_rule ~changes:(Changeset.encode changes) ~when_:On_success ()]) - (* For some tests, it can be relevant to have a manual trigger. *) - @ - match manual with - | No -> [] - | Yes -> [job_rule ~when_:Manual ()] - | On_changes changes -> - [job_rule ~when_:Manual ~changes:(Changeset.encode changes) ()]) - type opam_package_group = Executable | All type opam_package = { @@ -362,8 +318,50 @@ let build_debian_packages_image = (* Encodes the conditional [before_merging] pipeline and its unconditional variant [schedule_extended_test]. *) let jobs pipeline_type = - let make_rules ?label ?changes ?manual ?dependent () = - make_rules ?label ?changes ?manual ?dependent pipeline_type + (* [make_rules] makes rules for jobs that are: + - automatic in scheduled pipelines; + - conditional in [before_merging] pipelines. + + If a job has non-optional dependencies, then [dependent] must be + set to [true] to ensure that we only run the job in case previous + jobs succeeded (setting [when: on_success]). + + If [label] is set, add rule that selects the job in + [Before_merging] pipelines for merge requests with the given + label. Rules for manual triggers can be configured using + [manual]. + + If [label], [changes] and [manual] are omitted, then rules will + enable the job [On_success] in the [before_merging] + pipeline. This is safe, but prefer specifying a [changes] clause + if possible. *) + let make_rules ?label ?changes ?(manual = No) ?(dependent = false) () = + match pipeline_type with + | Schedule_extended_test -> + (* The scheduled pipeline always runs all jobs unconditionally + -- unless they are dependent on a previous, non-trigger job, in the + pipeline. *) + [job_rule ~when_:(if dependent then On_success else Always) ()] + | Before_merging -> ( + (* MR labels can be used to force tests to run. *) + (match label with + | Some label -> + [job_rule ~if_:Rules.(has_mr_label label) ~when_:On_success ()] + | None -> []) + (* Modifying some files can force tests to run. *) + @ (match changes with + | None -> [] + | Some changes -> + [ + job_rule ~changes:(Changeset.encode changes) ~when_:On_success (); + ]) + (* For some tests, it can be relevant to have a manual trigger. *) + @ + match manual with + | No -> [] + | Yes -> [job_rule ~when_:Manual ()] + | On_changes changes -> + [job_rule ~when_:Manual ~changes:(Changeset.encode changes) ()]) in (* Externalization *) let job_external_split ?(before_merging_suffix = "before_merging") -- GitLab From a659984c6eeb3f05b726d74f7f13abb02784d604 Mon Sep 17 00:00:00 2001 From: Arvid Jakobsson Date: Thu, 28 Mar 2024 12:30:23 +0100 Subject: [PATCH 2/5] CIAO: fix dependency check for jobs with no [needs:] --- ci/bin/tezos_ci.ml | 97 +++++++++++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 32 deletions(-) diff --git a/ci/bin/tezos_ci.ml b/ci/bin/tezos_ci.ml index 02ec244b20d8..fe1bd76ee530 100644 --- a/ci/bin/tezos_ci.ml +++ b/ci/bin/tezos_ci.ml @@ -151,45 +151,78 @@ module Pipeline = struct (* Check usage of [needs:] & [depends:] *) Fun.flip List.iter jobs @@ fun {job; _} -> (* Get the [needs:] / [dependencies:] of job *) - let opt_set l = String_set.of_list (Option.value ~default:[] l) in let needs = (* The mandatory set of needs *) - job.needs |> Option.value ~default:[] - |> List.filter_map (fun Gitlab_ci.Types.{job; optional} -> - if not optional then Some job else None) - |> String_set.of_list + match job.needs with + | Some needs -> + Some + (needs + |> List.filter_map (fun Gitlab_ci.Types.{job; optional} -> + if not optional then Some job else None) + |> String_set.of_list) + | None -> None in - let dependencies = opt_set job.dependencies in + let dependencies = + String_set.of_list (Option.value ~default:[] job.dependencies) + in + (* Check that jobs in [needs:]/[dependencies:] jobs are defined *) + (let dep_needs = + String_set.union + dependencies + (Option.value ~default:String_set.empty needs) + in + Fun.flip String_set.iter dep_needs @@ fun need -> + match Hashtbl.find_opt job_by_name need with + | Some _needed_job -> + (* TODO: https://gitlab.com/tezos/tezos/-/issues/7015 + check rule implication *) + () + | None -> + failwith + "[%s] job '%s' has a need on '%s' which is not defined in this \ + pipeline." + pipeline_name + job.name + need) ; (* Check that dependencies are a subset of needs. Note: this is already enforced by the smart constructor {!job} defined below. Is it redundant? Nothing enforces the usage of this smart constructor at this point.*) - String_set.iter - (fun dependency -> - if not (String_set.mem dependency needs) then - failwith - "[%s] the job '%s' has a [dependency:] on '%s' which is not \ - included in it's [need:]" - pipeline_name - job.name - dependency) - dependencies ; - (* Check that needed jobs (which thus includes dependencies) are defined *) - ( Fun.flip String_set.iter needs @@ fun need -> - match Hashtbl.find_opt job_by_name need with - | Some _needed_job -> - (* TODO: https://gitlab.com/tezos/tezos/-/issues/7015 - check rule implication *) - () - | None -> - (* TODO: https://gitlab.com/tezos/tezos/-/issues/7015 - handle optional needs *) - failwith - "[%s] job '%s' has a need on '%s' which is not defined in this \ - pipeline." - pipeline_name - job.name - need ) ; + (match needs with + | Some needs -> + String_set.iter + (fun dependency -> + if not (String_set.mem dependency needs) then + failwith + "[%s] the job '%s' has a [dependency:] on '%s' which is not \ + included in it's [need:]" + pipeline_name + job.name + dependency) + dependencies + | None -> + (* If the job has no needs, then it suffices that the dependency is in + an anterior stage. *) + let stage_index = + let stage_names = Stage.to_string_list () in + fun stage_opt -> + let stage = Option.value ~default:"test" stage_opt in + List.assoc_opt + stage + (List.combine stage_names (range 1 (List.length stage_names))) + in + String_set.iter + (fun dependency -> + (* We use [find] instead of [find_opt] *) + let dependency_job = Hashtbl.find job_by_name dependency in + if stage_index dependency_job.stage >= stage_index job.stage then + failwith + "[%s] the job '%s' has a [dependency:] on '%s' which is not in \ + a anterior stage." + pipeline_name + job.name + dependency) + dependencies) ; (* Check that all [dependencies:] are on jobs that produce artifacts *) ( Fun.flip String_set.iter dependencies @@ fun dependency -> match Hashtbl.find_opt job_by_name dependency with -- GitLab From 5556301872412e7c6135c90c99aabff1c8e4d3ad Mon Sep 17 00:00:00 2001 From: Arvid Jakobsson Date: Tue, 9 Apr 2024 15:14:08 +0200 Subject: [PATCH 3/5] CI: generate [oc.unified_coverage-before_merging.yml] and clean up Note the diff in dependencies. Actually, there is no need to spell out each instance of parallel jobs. Just giving the base of the name automatically makes the job receive the artifacts from all instances of the job. Also remove the file [coverage/common.yml] which is no longer used by any jobs. --- .gitlab/ci/jobs/coverage/common.yml | 17 --- .../oc.unified_coverage-before_merging.yml | 140 ++++++------------ ci/bin/code_verification.ml | 78 ++++++++-- ci/bin/common.ml | 14 -- ci/bin/main.ml | 1 - 5 files changed, 113 insertions(+), 137 deletions(-) delete mode 100644 .gitlab/ci/jobs/coverage/common.yml diff --git a/.gitlab/ci/jobs/coverage/common.yml b/.gitlab/ci/jobs/coverage/common.yml deleted file mode 100644 index 1ef60e31f25d..000000000000 --- a/.gitlab/ci/jobs/coverage/common.yml +++ /dev/null @@ -1,17 +0,0 @@ -.oc.template__coverage_report: - extends: .oc.template__coverage_location - stage: test_coverage - coverage: '/Coverage: ([^%]+%)/' - variables: - SLACK_COVERAGE_CHANNEL: "C02PHBE7W73" - artifacts: - expose_as: 'Coverage report' - reports: - coverage_report: - coverage_format: cobertura - path: _coverage_report/cobertura.xml - paths: - - _coverage_report/ - - $BISECT_FILE - expire_in: 15 days - when: always diff --git a/.gitlab/ci/jobs/coverage/oc.unified_coverage-before_merging.yml b/.gitlab/ci/jobs/coverage/oc.unified_coverage-before_merging.yml index ed7104fe12ee..57652428fe51 100644 --- a/.gitlab/ci/jobs/coverage/oc.unified_coverage-before_merging.yml +++ b/.gitlab/ci/jobs/coverage/oc.unified_coverage-before_merging.yml @@ -1,101 +1,49 @@ ---- -# This job fetches coverage files by precedent test stage. It creates the html, -# summary and cobertura reports. It also provide a coverage % for the merge request. - -include: .gitlab/ci/jobs/coverage/common.yml +# This file was automatically generated, do not edit. +# Edit file ci/bin/main.ml instead. oc.unified_coverage: - extends: - - .default_settings_template - - .image_template__runtime_e2etest_dependencies - - .oc.template__coverage_report - # We do not run this job when margebot triggers the - # pipeline. Instead, the coverage traces are downloaded in the - # master pipeline and the report is computed there. - - .rules__octez_changes_and_not_margebot - variables: - # This inhibites the Makefile's opam version check, which this - # job's opam-less image cannot pass. - TEZOS_WITHOUT_OPAM: "true" - # This job requires all bisect_ppx artifacts from the stage test, so we override - # the `dependencies: []` in `.default_settings` with a list of jobs. - # Each new job in the stage test needs to be manually added to this list. - # /!\ Warning: this list is read by `scripts/ci/download_coverage/download.ml`. - # The 'dependencies' field must be followed directly by the 'script' field. + image: ${build_deps_image_name}:runtime-e2etest-dependencies--${build_deps_image_version} + stage: test_coverage + tags: + - gcp + rules: + - if: $GITLAB_USER_LOGIN == "nomadic-margebot" + when: never + - changes: + - .gitlab-ci.yml + - .gitlab/**/* + - etherlink/**/* + - michelson_test_scripts/**/* + - src/**/* + - tezt/**/* + - tzt_reference_test_suite/**/* + when: on_success dependencies: - - "tezt 1/60" - - "tezt 2/60" - - "tezt 3/60" - - "tezt 4/60" - - "tezt 5/60" - - "tezt 6/60" - - "tezt 7/60" - - "tezt 8/60" - - "tezt 9/60" - - "tezt 10/60" - - "tezt 11/60" - - "tezt 12/60" - - "tezt 13/60" - - "tezt 14/60" - - "tezt 15/60" - - "tezt 16/60" - - "tezt 17/60" - - "tezt 18/60" - - "tezt 19/60" - - "tezt 20/60" - - "tezt 21/60" - - "tezt 22/60" - - "tezt 23/60" - - "tezt 24/60" - - "tezt 25/60" - - "tezt 26/60" - - "tezt 27/60" - - "tezt 28/60" - - "tezt 29/60" - - "tezt 30/60" - - "tezt 31/60" - - "tezt 32/60" - - "tezt 33/60" - - "tezt 34/60" - - "tezt 35/60" - - "tezt 36/60" - - "tezt 37/60" - - "tezt 38/60" - - "tezt 39/60" - - "tezt 40/60" - - "tezt 41/60" - - "tezt 42/60" - - "tezt 43/60" - - "tezt 44/60" - - "tezt 45/60" - - "tezt 46/60" - - "tezt 47/60" - - "tezt 48/60" - - "tezt 49/60" - - "tezt 50/60" - - "tezt 51/60" - - "tezt 52/60" - - "tezt 53/60" - - "tezt 54/60" - - "tezt 55/60" - - "tezt 56/60" - - "tezt 57/60" - - "tezt 58/60" - - "tezt 59/60" - - "tezt 60/60" - - "tezt-memory-4k 1/4" - - "tezt-memory-4k 2/4" - - "tezt-memory-4k 3/4" - - "tezt-memory-4k 4/4" - - "tezt-memory-3k" - - "tezt-time-sensitive" - - "oc.unit:non-proto-x86_64" - - "oc.unit:proto-x86_64" - - "oc.unit:other-x86_64" - script: - # On the development branches, we compute coverage. - # TODO: https://gitlab.com/tezos/tezos/-/issues/6173 - # We propagate the exit code to temporarily allow corrupted coverage files. - - ./scripts/ci/report_coverage.sh || exit $? + - oc.unit:non-proto-x86_64 + - oc.unit:other-x86_64 + - oc.unit:proto-x86_64 + - tezt-flaky + - tezt + - tezt-memory-4k + - tezt-memory-3k + - tezt-time-sensitive allow_failure: exit_codes: 64 + script: + - ./scripts/ci/report_coverage.sh || exit $? + variables: + TEZOS_WITHOUT_OPAM: "true" + BISECT_FILE: $CI_PROJECT_DIR/_coverage_output/ + SLACK_COVERAGE_CHANNEL: C02PHBE7W73 + artifacts: + expire_in: 15 days + paths: + - _coverage_report/ + - $BISECT_FILE + reports: + coverage_report: + coverage_format: cobertura + path: _coverage_report/cobertura.xml + when: always + expose_as: Coverage report + coverage: '/Coverage: ([^%]+%)/' diff --git a/ci/bin/code_verification.ml b/ci/bin/code_verification.ml index eb55986dba7e..01d838a869d6 100644 --- a/ci/bin/code_verification.ml +++ b/ci/bin/code_verification.ml @@ -332,10 +332,13 @@ let jobs pipeline_type = [manual]. If [label], [changes] and [manual] are omitted, then rules will - enable the job [On_success] in the [before_merging] - pipeline. This is safe, but prefer specifying a [changes] clause - if possible. *) - let make_rules ?label ?changes ?(manual = No) ?(dependent = false) () = + enable the job [On_success] in the [before_merging] pipeline. This + is safe, but prefer specifying a [changes] clause if possible. + + If [margebot_disable] is set to true (default false), this job is + disabled when marge-bot triggers the [before_merging] pipeline. *) + let make_rules ?label ?changes ?(manual = No) ?(dependent = false) + ?(margebot_disable = false) () = match pipeline_type with | Schedule_extended_test -> (* The scheduled pipeline always runs all jobs unconditionally @@ -344,10 +347,13 @@ let jobs pipeline_type = [job_rule ~when_:(if dependent then On_success else Always) ()] | Before_merging -> ( (* MR labels can be used to force tests to run. *) - (match label with - | Some label -> - [job_rule ~if_:Rules.(has_mr_label label) ~when_:On_success ()] - | None -> []) + (if margebot_disable then + [job_rule ~if_:Rules.triggered_by_marge_bot ~when_:Never ()] + else []) + @ (match label with + | Some label -> + [job_rule ~if_:Rules.(has_mr_label label) ~when_:On_success ()] + | None -> []) (* Modifying some files can force tests to run. *) @ (match changes with | None -> [] @@ -400,6 +406,22 @@ let jobs pipeline_type = in (* Common GitLab CI caches *) let cache_kernels = {key = "kernels"; paths = ["cargo/"]} in + (* Collect coverage trace producing jobs *) + let jobs_with_coverage_output = ref [] in + (* Add variables for bisect_ppx output and store the traces as an + artifact. + + This function should be applied to test jobs that produce coverage. *) + let enable_coverage_output_artifact ?(expire_in = Duration (Days 1)) tezos_job + : tezos_job = + jobs_with_coverage_output := tezos_job :: !jobs_with_coverage_output ; + tezos_job |> enable_coverage_location + |> add_artifacts + ~expire_in + ~name:"coverage-files-$CI_JOB_ID" + ~when_:On_success + ["$BISECT_FILE"] + in (* Stages *) (* All stages should be empty, as explained below, until the full pipeline is generated. *) let trigger_stage, make_dependencies = @@ -1649,6 +1671,43 @@ let jobs pipeline_type = [job_commit_titles] | Schedule_extended_test -> [] in + let coverage = + match pipeline_type with + | Before_merging -> + (* This job fetches coverage files by precedent test stage. It creates + the html, summary and cobertura reports. It also provide a coverage % + for the merge request. *) + let job_unified_coverage : tezos_job = + let dependencies = List.rev !jobs_with_coverage_output in + job + ~__POS__ + ~image:Images.runtime_e2etest_dependencies + ~name:"oc.unified_coverage" + ~stage:Stages.test_coverage + ~coverage:"/Coverage: ([^%]+%)/" + ~rules: + (make_rules ~margebot_disable:true ~changes:changeset_octez ()) + ~variables: + [ + (* This inhibits the Makefile's opam version check, which + this job's opam-less image + ([runtime_e2etest_dependencies]) cannot pass. *) + ("TEZOS_WITHOUT_OPAM", "true"); + ] + ~dependencies:(Staged dependencies) + (* On the development branches, we compute coverage. + TODO: https://gitlab.com/tezos/tezos/-/issues/6173 + We propagate the exit code to temporarily allow corrupted coverage files. *) + (script_propagate_exit_code "./scripts/ci/report_coverage.sh") + ~allow_failure:(With_exit_codes [64]) + |> enable_coverage_location |> enable_coverage_report + |> job_external + ~directory:"coverage" + ~filename_suffix:"before_merging" + in + [job_unified_coverage] + | Schedule_extended_test -> [] + in let doc = let jobs_install_python = (* Creates a job that tests installation of the python environment in [image] *) @@ -1889,5 +1948,6 @@ let jobs pipeline_type = (using {!job_external} or {!jobs_external}) and included by hand in the files [.gitlab/ci/pipelines/before_merging.yml] and [.gitlab/ci/pipelines/schedule_extended_test.yml]. *) - ignore (trigger_stage @ sanity @ build @ packaging @ test @ doc @ manual) ; + ignore + (trigger_stage @ sanity @ build @ packaging @ test @ coverage @ doc @ manual) ; [] diff --git a/ci/bin/common.ml b/ci/bin/common.ml index 09c7fd728472..06253e69df20 100644 --- a/ci/bin/common.ml +++ b/ci/bin/common.ml @@ -256,20 +256,6 @@ let enable_coverage_location : tezos_job -> tezos_job = Tezos_ci.append_variables [("BISECT_FILE", "$CI_PROJECT_DIR/_coverage_output/")] -(** Add variables for bisect_ppx output and store the traces as an - artifact. - - This function should be applied to test jobs that produce coverage. *) -let enable_coverage_output_artifact ?(expire_in = Duration (Days 1)) : - tezos_job -> tezos_job = - fun job -> - job |> enable_coverage_location - |> Tezos_ci.add_artifacts - ~expire_in - ~name:"coverage-files-$CI_JOB_ID" - ~when_:On_success - ["$BISECT_FILE"] - let enable_coverage_report job : tezos_job = job |> Tezos_ci.add_artifacts diff --git a/ci/bin/main.ml b/ci/bin/main.ml index 495414f3b95c..47c9f16deed3 100644 --- a/ci/bin/main.ml +++ b/ci/bin/main.ml @@ -186,7 +186,6 @@ let () = to CI-in-OCaml, they should be removed from this function *) let exclude = function | ".gitlab/ci/jobs/coverage/common.yml" - | ".gitlab/ci/jobs/coverage/oc.unified_coverage-before_merging.yml" | ".gitlab/ci/jobs/shared/images.yml" | ".gitlab/ci/jobs/shared/templates.yml" | ".gitlab/ci/jobs/test/common.yml" | ".gitlab/ci/pipelines/before_merging.yml" -- GitLab From a6de86a78ba981989e05e975e6755883086f516e Mon Sep 17 00:00:00 2001 From: Arvid Jakobsson Date: Fri, 5 Apr 2024 15:39:37 +0200 Subject: [PATCH 4/5] [scripts]: clapify [download_coverage/download.ml] an importance piece of maintenance work. --- .gitlab/ci/pipelines/master_branch.yml | 2 +- ci/bin/master_branch.ml | 4 +-- scripts/ci/download_coverage/download.ml | 45 ++++++++++++++++-------- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/.gitlab/ci/pipelines/master_branch.yml b/.gitlab/ci/pipelines/master_branch.yml index 1a00e72b5e2a..754dbd2a24f4 100644 --- a/.gitlab/ci/pipelines/master_branch.yml +++ b/.gitlab/ci/pipelines/master_branch.yml @@ -215,7 +215,7 @@ oc.unified_coverage: - . ./scripts/version.sh script: - mkdir -p _coverage_report - - dune exec scripts/ci/download_coverage/download.exe -- -a from=last-merged-pipeline + - dune exec scripts/ci/download_coverage/download.exe -- --from last-merged-pipeline --info --log-file _coverage_report/download_coverage.log - ./scripts/ci/report_coverage.sh variables: diff --git a/ci/bin/master_branch.ml b/ci/bin/master_branch.ml index c364f1282cb9..c54b8ebe932d 100644 --- a/ci/bin/master_branch.ml +++ b/ci/bin/master_branch.ml @@ -98,8 +98,8 @@ let jobs = [ (* On the project default branch, we fetch coverage from the last merged MR *) "mkdir -p _coverage_report"; - "dune exec scripts/ci/download_coverage/download.exe -- -a \ - from=last-merged-pipeline --info --log-file \ + "dune exec scripts/ci/download_coverage/download.exe -- --from \ + last-merged-pipeline --info --log-file \ _coverage_report/download_coverage.log"; "./scripts/ci/report_coverage.sh"; ] diff --git a/scripts/ci/download_coverage/download.ml b/scripts/ci/download_coverage/download.ml index 2f7a50955114..26d9885294b9 100644 --- a/scripts/ci/download_coverage/download.ml +++ b/scripts/ci/download_coverage/download.ml @@ -2,15 +2,17 @@ open Tezt open Base open Tezt_gitlab -let usage () = - prerr_endline - {|Usage: dune exec scripts/ci/download_coverage/download.exe -- [-a from=last-merged-pipeline | -a from=] +let section = + Clap.section + "DOWNLOAD COVERAGE TRACES" + ~description: + {|Download coverage traces from a pipeline. Example: to fetch coverage traces from https://gitlab.com/tezos/tezos/-/pipelines/426773806, run (from the root of the repository): -dune exec scripts/ci/download_coverage/download.exe -- -a from=426773806 +dune exec scripts/ci/download_coverage/download.exe -- --from 426773806 You can use the PROJECT environment variable to specify which GitLab repository to fetch coverage traces from. Default is: tezos/tezos @@ -19,10 +21,9 @@ The script can also be used to fetch coverage traces from the last successful pi latest MR merged to the default branch (configurable through the DEFAULT_BRANCH environment variable) for a given PROJECT: -dune exec scripts/ci/download_coverage/download.exe -- -a from=last-merged-pipeline +dune exec scripts/ci/download_coverage/download.exe -- --from last-merged-pipeline -|} ; - exit 1 +|} let project = Sys.getenv_opt "PROJECT" |> Option.value ~default:"tezos/tezos" @@ -99,12 +100,28 @@ let fetch_pipeline_coverage_from_jobs pipeline = type from = Pipeline of int | Last_merged_pipeline -let cli_get_from = - match Cli.get_string_opt "from" with - | Some "last-merged-pipeline" -> Last_merged_pipeline - | Some s -> ( - match int_of_string_opt s with Some i -> Pipeline i | None -> usage ()) - | None -> usage () +let cli_from_type = + let parse = function + | "last-merged-pipeline" -> Some Last_merged_pipeline + | s -> Option.map (fun x -> Pipeline x) (int_of_string_opt s) + in + let show = function + | Pipeline id -> string_of_int id + | Last_merged_pipeline -> "last-merged-pipeline" + in + Clap.typ ~name:"from" ~dummy:Last_merged_pipeline ~parse ~show + +let cli_from = + Clap.default + cli_from_type + ~section + ~long:"from" + ~placeholder:"PIPELINE" + ~description: + "The ID of the pipeline to fetch records from. Also accepts \ + 'last-merged-pipeline', which denotes the last pipeline for the latest \ + merge commit on the default branch." + Last_merged_pipeline let () = (* Register a test to benefit from error handling of Test.run, @@ -112,7 +129,7 @@ let () = ( Test.register ~__FILE__ ~title:"download coverage" ~tags:["update"] @@ fun () -> let* _new_coverage_traces = - match cli_get_from with + match cli_from with | Pipeline pipeline_id -> fetch_pipeline_coverage_from_jobs pipeline_id | Last_merged_pipeline -> let* pipeline_id = -- GitLab From b874c80807853d9f5805918b6e3488d3b8613842 Mon Sep 17 00:00:00 2001 From: Arvid Jakobsson Date: Fri, 5 Apr 2024 15:40:05 +0200 Subject: [PATCH 5/5] [scripts]: adapt [download_coverage/download.ml] The CI generator now writes the file [script-inputs/ci-coverage-producing-jobs], and the [download.ml] script now reads this file instead to find the jobs that produce coverage traces. --- ci/bin/code_verification.ml | 10 ++++++ ci/bin/tezos_ci.ml | 2 ++ ci/bin/tezos_ci.mli | 9 +++++ script-inputs/ci-coverage-producing-jobs | 8 +++++ scripts/ci/download_coverage/download.ml | 43 +++++++++++------------- scripts/tezt_gitlab/gitlab.ml | 10 ++++-- 6 files changed, 57 insertions(+), 25 deletions(-) create mode 100644 script-inputs/ci-coverage-producing-jobs diff --git a/ci/bin/code_verification.ml b/ci/bin/code_verification.ml index 01d838a869d6..fa0c6f2ffb6d 100644 --- a/ci/bin/code_verification.ml +++ b/ci/bin/code_verification.ml @@ -1674,6 +1674,16 @@ let jobs pipeline_type = let coverage = match pipeline_type with | Before_merging -> + (* Write the name of each job that produces coverage as input for other scripts. + Only includes the stem of the name: parallel jobs only appear once. + E.g. as [tezt], not [tezt 1/60], [tezt 2/60], etc. *) + Base.write_file + "script-inputs/ci-coverage-producing-jobs" + ~contents: + (String.concat + "\n" + (List.map Tezos_ci.name_of_tezos_job !jobs_with_coverage_output) + ^ "\n") ; (* This job fetches coverage files by precedent test stage. It creates the html, summary and cobertura reports. It also provide a coverage % for the merge request. *) diff --git a/ci/bin/tezos_ci.ml b/ci/bin/tezos_ci.ml index fe1bd76ee530..f546538ba005 100644 --- a/ci/bin/tezos_ci.ml +++ b/ci/bin/tezos_ci.ml @@ -53,6 +53,8 @@ type tezos_job = { source_position : string * int * int * int; } +let name_of_tezos_job tezos_job = tezos_job.job.name + let map_job (tezos_job : tezos_job) (f : Gitlab_ci.Types.job -> Gitlab_ci.Types.job) : tezos_job = {tezos_job with job = f tezos_job.job} diff --git a/ci/bin/tezos_ci.mli b/ci/bin/tezos_ci.mli index c92d3958eaad..f5d40afb757d 100644 --- a/ci/bin/tezos_ci.mli +++ b/ci/bin/tezos_ci.mli @@ -8,6 +8,15 @@ (** A GitLab CI job annotated with Octez-specific meta-data. *) type tezos_job +(** The name of a {!tezos_job} as given to [~name] of {!job}. + + This returns the stem of the job name. Parallel jobs produce + multiple job instances ([JOB N/M] for a {!Vector}-parallel job and + [JOB: [foo, bar, ...]] for {!Matrix}-parallel jobs) -- the stem of such + jobs is [JOB]. For non-parallel jobs, the argument given to + [~name] and the stem is equivalent. *) +val name_of_tezos_job : tezos_job -> string + (** A string that should be prepended to all generated files. Warns not to modify the generated files, and refers to the generator. *) diff --git a/script-inputs/ci-coverage-producing-jobs b/script-inputs/ci-coverage-producing-jobs new file mode 100644 index 000000000000..f1d6e4145837 --- /dev/null +++ b/script-inputs/ci-coverage-producing-jobs @@ -0,0 +1,8 @@ +tezt-time-sensitive +tezt-memory-3k +tezt-memory-4k +tezt +tezt-flaky +oc.unit:proto-x86_64 +oc.unit:other-x86_64 +oc.unit:non-proto-x86_64 diff --git a/scripts/ci/download_coverage/download.ml b/scripts/ci/download_coverage/download.ml index 26d9885294b9..30658f6dda0d 100644 --- a/scripts/ci/download_coverage/download.ml +++ b/scripts/ci/download_coverage/download.ml @@ -33,34 +33,20 @@ let default_branch = let coverage_traces_directory = Sys.getenv_opt "COVERAGE_OUTPUT" |> Option.value ~default:"_coverage_output" -let coverage_yml_path = - ".gitlab/ci/jobs/coverage/oc.unified_coverage-before_merging.yml" +let coverage_jobs_path = "script-inputs/ci-coverage-producing-jobs" -(** Read {!coverage_yml_path} to find the set of - jobs from which to collect coverage traces *) +(** Read {!coverage_jobs_path} to find the set of jobs from which to + collect coverage traces *) let coverage_jobs = - let coverage_yml = - Base.read_file coverage_yml_path |> String.split_on_char '\n' - in - let _, coverage_yml = - Base.span (fun line -> not (line =~ rex "^\\s+dependencies:$")) coverage_yml - in - let coverage_jobs, _ = - Base.span (fun line -> not (line =~ rex "^\\s+script:$")) coverage_yml - in + (* Note: the contents of this file only includes the stem of each job name. *) let coverage_jobs = - List.map - (function - | line -> ( - match line =~* rex {|- "(.*)"|} with - | Some job_name -> job_name - | None -> - Test.fail "Unexpected line %S in %s" line coverage_yml_path)) - (List.tl coverage_jobs) + Base.read_file coverage_jobs_path + |> String.split_on_char '\n' + |> List.filter (( <> ) "") in Log.debug "From %s, read coverage jobs: [%s]" - coverage_yml_path + coverage_jobs_path (String.concat ", " coverage_jobs) ; coverage_jobs @@ -79,6 +65,7 @@ let fetch_pipeline_coverage_from_jobs pipeline = let get_coverage_trace job = let job_id = JSON.(job |-> "id" |> as_int) in let job_name = JSON.(job |-> "name" |> as_string) in + let job_status = JSON.(job |-> "status" |> as_string) in let artifact_name = Base.replace_string (rex "[\\\\/_ @\\[\\]]+") job_name ~by:"-" ^ ".coverage" @@ -86,7 +73,17 @@ let fetch_pipeline_coverage_from_jobs pipeline = job_names are mangled into coverage trace artifact paths. *) in let artifact_path = coverage_traces_directory // artifact_name in - if List.mem job_name coverage_jobs then + if + (job_status = "success" || job_status = "failed") + && List.exists + (fun coverage_job_stem -> + job_name = coverage_job_stem + (* vector parallel *) + || job_name =~ rex ("^" ^ coverage_job_stem ^ " \\d+/\\d+$") + || (* matrix parallel *) + job_name =~ rex ("^" ^ coverage_job_stem ^ ": \\[.*\\]$")) + coverage_jobs + then Some ( Gitlab.project_job_artifact ~project ~job_id ~artifact_path (), job_name, diff --git a/scripts/tezt_gitlab/gitlab.ml b/scripts/tezt_gitlab/gitlab.ml index 3281a5db1d11..cf0fc38f14cd 100644 --- a/scripts/tezt_gitlab/gitlab.ml +++ b/scripts/tezt_gitlab/gitlab.ml @@ -32,13 +32,19 @@ let project_job_artifact ~project ~job_id ~artifact_path = job_id artifact_path) -let curl_params ?(fail_on_http_errors = true) ?output_path ?(location = false) - uri = +let curl_params ?(progress_meter = false) ?(fail_on_http_errors = true) + ?output_path ?(location = false) uri = (if fail_on_http_errors then ["--fail"] else []) @ (match output_path with | Some output_path -> ["--output"; output_path] | None -> []) @ (if location then ["--location"] else []) + @ (if + progress_meter + || Sys.getenv_opt "MY_CURL_VERSION_IS" + |> Option.value ~default:"" = "very_very_old" + then [] + else ["--no-progress-meter"]) @ [Uri.to_string uri] let get ?fail_on_http_errors uri = -- GitLab