From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 8E399A0F66E; Mon, 4 Mar 2024 19:49:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8E399A0F66E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1709570955; bh=1hSqUJYSQqHdO/YC6F5CFXavjkuOkuXLilM32eARAdE=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=zPd/bMvikEjSBXLC+UH1G/09MBlxc/L0LsaXwkRf8T0niQgZl4pDs0p53Yz1HfQI/ mFhpRDBoebqGPEsgTokQjS2YF21rPkR3neJsvCN5Wy2HPHDGHHjjNG04niPyQXxIDE sgYo74xYBfkafCiJ+zcPoOYXDDWhZ5PRDzplgfrU= Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [95.163.41.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 08AB6A0F640 for ; Mon, 4 Mar 2024 19:49:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 08AB6A0F640 Received: by smtp59.i.mail.ru with esmtpa (envelope-from ) id 1rhBV2-000000010u0-39IJ; Mon, 04 Mar 2024 19:49:13 +0300 Date: Mon, 4 Mar 2024 19:45:14 +0300 To: Maxim Kokryashkin Message-ID: References: <20240301082538.107373-1-m.kokryashkin@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240301082538.107373-1-m.kokryashkin@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD987C0EE6E7F0A597DF2E6F6001F0923FF54E1DF110EA51DBE182A05F53808504063054D220D4347F79487ABAC94A94B54C33B5A886C657E8D1105BA0FE663C68389BD80B6B4489A2D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE737BB76880A4CA9A4EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006372D9C4B6214E2BA508638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D89F7587A0F2513B5BA23F3870F80FE7B077C233A4B385ACA9CC7F00164DA146DAFE8445B8C89999728AA50765F7900637D0FEED2715E18529389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC81D471462564A2E19F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE33AC447995A7AD182BEBFE083D3B9BA73A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E7356C9A9530EBF72002C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A5E033EFD0590666A75002B1117B3ED696891CEB3FEC17427E72305013E4AE841E823CB91A9FED034534781492E4B8EEAD2B25D9E4C92BC8ACBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF1325D4C8A2278FC9D57FDC40FD1442887A2DED0B568957748E025D5F0A67F2A408FC8DA71D385488A2A143A52978B9DD53D27135B3F37DBBEFC83F2529E51CC6FD52F8B44F2BD9ECC226CC413062362A913E6812662D5F2A5EAB5682573093F7837F15F2B5E4A70B33F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj+F5rnR9AEqn4T8os6jkqWQ== X-DA7885C5: 2EA3EDF8F59E57C5F255D290C0D534F96C80270E578CFD7CCA41107B2790B6982B5C3F31497A89315B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F7393590D8C940224AE33D2665E6B957531F46A8446BC877890EF1525788111AB4F73E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] ci: extend tarantool integration testing X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Maxim! Thanks for the patch! Please consider my comments below. On 01.03.24, Maxim Kokryashkin wrote: > This patch makes use of reusable workflows from the > Tarantool repository, so now LuaJIT CI tests integration > in all relevant workflows. > > Also, this patch disables MacOS testing for Tarantool > integration since those runs were made nightly in the > Tarantool repo. > > As noted in the documentation[1], it is impossible for a single Typo: s/[1]/ [1]/ > workflow call tree to include more than 20 workflows. To > overcome this limitation, workflows are started in a bunch of > separate files. This makes it impossible to depend on LuaJIT-only > jobs for integration workflow since it is impossible to make > cross-dependencies between workflow files. > > The name of a callee workflow cannot be substituted using a > matrix[2], so workflow calls are copied and pasted instead. Please add the corresponding comment to the workflow file too. Typo: s/[2]/ [2]/ > > Table below shows which workflows are enabled and why. > > Workflow name |+/-| Reason > ----------------------------------------------------------------- > default_gcc_centos_7 | + | Ancient gcc build. Agree with Sergey here -- "gcc version (7)" looks better. > luajit-integration | + | Exotic LuaJIT options. > memtx_allocator_based_on_malloc | + | Not relevant to LuaJIT. Typo: s/+/-/ > osx | - | Nightly build. > out_of_source | - | Not relevent to LuaJIT. I don't agree here. We want to check that our tests work in OOS builds too. It may be an issue for some libraries to be built or for some tests that use generated files, etc. If we enable this, we can skip static_build_cmake_linux workflow, as Sergey suggested (since it is the same as static_build, except it is OOS build). But I'm not against too leave that workflow as well. Also, looks like it is used, see below [*]. > static_build | + | Tarantool static build. > static_build_cmake_linux | + | Tarantool static build. > [1]: https://docs.github.com/en/actions/using-workflows/reusing-workflows#limitations > [2]: https://github.com/orgs/community/discussions/45342#discussioncomment-4779360 > --- > Branch: https://github.com/tarantool/luajit/tree/fckxorg/integration-testing > > .github/workflows/exotic-tarantool.yml | 62 ++++++++++++ > .github/workflows/tarantool-ecosystem.yml | 35 +++++++ > .github/workflows/testing.yml | 116 ++++++++++++++++------ > 3 files changed, 183 insertions(+), 30 deletions(-) > create mode 100644 .github/workflows/exotic-tarantool.yml > create mode 100644 .github/workflows/tarantool-ecosystem.yml > > diff --git a/.github/workflows/exotic-tarantool.yml b/.github/workflows/exotic-tarantool.yml > new file mode 100644 > index 00000000..be55291a > --- /dev/null > +++ b/.github/workflows/exotic-tarantool.yml Minor: I suggest using tarantool-exoting to be consistent with tarantool-ecosystem. Within this rename, don't forget to rename the job for consistency. OTOH, maybe it is better to name this workflow like the tarantool-integration or luajit-integration to show that we run **LuaJIT** tests under Tarantool. What name do you like the most? > +jobs: > + test-exotic-tarantool: Why do you separate this job from the testing.yml workflow? Also, it should depend on luajit-testing, so it still should be placed in the shouldn't it? If not, please add a rationale in the comment. > + strategy: > + fail-fast: false > + matrix: > + ARCH: [ARM64, x86_64] > + BUILDTYPE: [Debug, Release] > + GC64: [ON, OFF] > + OS: [Linux, macOS] > + include: > + - BUILDTYPE: Debug > + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug > + - BUILDTYPE: Release > + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo > + exclude: > + - ARCH: ARM64 > + GC64: OFF > + - OS: macOS > + GC64: OFF > + name: > > + Tarantool > + (${{ matrix.OS }}/${{ matrix.ARCH }}) > + ${{ matrix.BUILDTYPE }} > + GC64:${{ matrix.GC64 }} > + # XXX: Only LuaJIT-tests are running in this workflow. Side note: What do you think about adding the FreeBSD runs to that workflow? It doesn't cost much since LuaJIT-tests are tiny. And we already run such tests for OSX. > + uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master > + with: > + CMAKE_EXTRA_PARAMS: > > + -G Ninja > + ${{ matrix.CMAKEFLAGS }} > + -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }} > + arch: ${{ matrix.ARCH }} > + os: ${{ matrix.OS }} > + revision: ${{ github.sha }} > diff --git a/.github/workflows/tarantool-ecosystem.yml b/.github/workflows/tarantool-ecosystem.yml > new file mode 100644 > index 00000000..82a47fe1 > --- /dev/null > +++ b/.github/workflows/tarantool-ecosystem.yml > +jobs: > + test-tarantool-integration: > + uses: tarantool/tarantool/.github/workflows/integration.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} Do I understand correctly that this workflow is separated from testing integration list since it has reached the limit mentioned above (20 workflows)? If so, please mention this here as a comment and in the testing.yml too. > + Nit: Excess empty line. > diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml > index cb4ba57b..7bd527be 100644 > --- a/.github/workflows/testing.yml > +++ b/.github/workflows/testing.yml > @@ -83,37 +83,93 @@ jobs: > run: cmake --build . --parallel --target LuaJIT-test > working-directory: ${{ env.BUILDDIR }} > > + test-tarantool-coverage: Maybe it is better to add names for better readablility. Something like Tarantool intergration coverage, here and below. > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/coverage.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + test-tarantool-debug: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/debug.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-debug_aarch64: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/debug_aarch64.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-debug_asan_clang: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/debug_asan_clang.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-default_gcc_centos_7: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/default_gcc_centos_7.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-out_of_source: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/out_of_source.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} [*] This workflow is mentioned as not used. > + > + test-tarantool-release: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/release.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-release_asan_clang: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/release_asan_clang.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-release_clang: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/release_clang.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-release_lto: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/release_lto.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-release_lto_clang: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/release_lto_clang.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-static_build: > + needs: test-luajit > + uses: tarantool/tarantool/.github/workflows/static_build.yml@master > + with: > + submodule: luajit > + revision: ${{ github.sha }} > + > + test-tarantool-static_build_cmake_linux: > needs: test-luajit > - uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master > + uses: tarantool/tarantool/.github/workflows/static_build_cmake_linux.yml@master > with: > - CMAKE_EXTRA_PARAMS: > > - -G Ninja > - ${{ matrix.CMAKEFLAGS }} > - -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }} > - arch: ${{ matrix.ARCH }} > - os: ${{ matrix.OS }} > + submodule: luajit > revision: ${{ github.sha }} > -- > 2.43.0 > -- Best regards, Sergey Kaplun