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 90F8A6ECE3; Thu, 30 Jun 2022 01:25:20 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 90F8A6ECE3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1656541520; bh=1DyqPyCdNRCv9KINzMVh//RvSpCE3Dt4yF5EAn9h7PU=; 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=x3mDISjn4G5kW9n7yHydG6UhBcx3WhNlPaYF37lcRDQeePv9fAbEydjoDG12qx124 JOLgOwEF0/SdpcmkjK3SYYvtShQ0jjSuvqf+3EcbdHHmb3HdGPDnuL+8iLHDtr41AN EJAVuopYDTZCA2cJiDBW/t4u5J1jglDhUyYLltiE= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 300E86ECE4 for ; Thu, 30 Jun 2022 01:24:57 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 300E86ECE4 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1o6g7E-0007tF-5S; Thu, 30 Jun 2022 01:24:56 +0300 Date: Thu, 30 Jun 2022 01:17:45 +0300 To: Sergey Kaplun Message-ID: References: <0b728799a1a9a03e008c249ed12ded71134c5bd9.1655911291.git.imun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9F05DDD76985F9126E1DDAE1C6EB0772846459A7F84F1DAA6182A05F5380850403C150CFEEE2726BE03B0186C6947C0B219934037775BEDB948F92316039AC5CF X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE70C5E0F71D77D667BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637CD86AAD37E7767A88638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8924A52BC19D4F586EBCC6B0B430BD255117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BF1175FABE1C0F9B6A471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CE0F3BA37685B2B9043847C11F186F3C59DAA53EE0834AAEE X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 9604B64F49C60606AD91A466A1DEF99B296C473AB1E14218C6CDE5D1141D2B1C7E62A60AC0974049550393E72E4AE07BCA5FC698F1E55B38AD91A466A1DEF99B296C473AB1E14218B936CB490224F2464EEA7BD89490CAC0EDDA962BC3F61961 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D346AD04FDA812EEFF1F1726BEC287941FDC69BF8760D8D8915F9E28064347AF1867A2E148D50C30CB01D7E09C32AA3244CA9B664FC8C83AC6F7DEFEC9E4368AAE4FE8DA44ABE2443F7927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojhvrbUd41KJlDkMZFMa5swg== X-Mailru-Sender: 689FA8AB762F7393CC2E0F076E87284E384167AB9263ADA5681D0C761821CE25A7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E3365FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 3/3] ci: add 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: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergey, Thanks for your review! On 23.06.22, Sergey Kaplun wrote: > Hi, Igor! > > Thanks for the patch! > Nice to see our CI improving! Glad this makes somebody a little happier :) > > Please consider my comments below. > > On 22.06.22, Igor Munkin wrote: > > This patch introduces additional steps with Tarantool integration > > testing to all existing LuaJIT workflows. All steps invoke reusable > > workflow[1] from Tarantool repository for each matrix entry being set > > for particular LuaJIT workflow. > > AFAICS, there are no workflows for different OS like Fedora, FreeBSD, > etc.. Same for integration testing workflow for Tarantool itself, plus > workflows with different compilers (gcc/clang). > > Should developer still bump LuaJIT in Tarantool repo and create PR to > test full Tarantool CI? Well, I'm trying my best to make tarantool/luajit CI as close to the one in tarantool/tarantool as it's possible. However, I'm afraid we will face two challenges soon: 1. As far as we're going to add some custom LuaJIT configurations testing and instrumenting testing to LuaJIT CI, it will expand already extensive Tarantool testing. 2. tarantool/tarantool is the main repository and we share runners with it, so we should not forget about already overloaded infrastructure. All in all we need to create the sufficient set of workflows that will alleviate LuaJIT developer suffering without dropping patches quality. If you're asking about this particular patch, then yes, you still need to create a draft PR in tarantool/tarantool to check if everything is fine. By the way, I hear many opinions whether developers should use 'full-ci' tag for PRs or not. If you don't use it, you can just run all pipelines in tarantool/luajit and leave everything else to a mainainer. Otherwise, there is no other option than draft PR (I believe you can create a single PR in tarantool/tarantool for a bunch of your patches, but I'm not a big fan of it). Anyway, here is the list for the upcoming enhancements: * FreeBSD workflow * Various compilers matrix (GCC, Clang) * JIT enabled/disabled matrix * Valgrind testing * luarocks v3 world testing > > > > > [1]: https://github.com/tarantool/tarantool/blob/master/.github/workflows/luajit-integration.yml > > > > Signed-off-by: Igor Munkin > > --- > > .github/workflows/linux-aarch64.yml | 25 +++++++++++++++-- > > .github/workflows/linux-x86_64.yml | 43 +++++++++++++++++++++++++++-- > > .github/workflows/macos-m1.yml | 25 +++++++++++++++-- > > .github/workflows/macos-x86_64.yml | 43 +++++++++++++++++++++++++++-- > > 4 files changed, 124 insertions(+), 12 deletions(-) > > > > diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml > > index de360b12..293b572e 100644 > > --- a/.github/workflows/linux-aarch64.yml > > +++ b/.github/workflows/linux-aarch64.yml > > @@ -1,4 +1,4 @@ > > -name: "LuaJIT test workflow (Linux/aarch64)" > > +name: "Linux/aarch64 test workflow" > > > > on: > > push: > > @@ -29,7 +29,7 @@ concurrency: > > cancel-in-progress: true > > > > jobs: > > - test-linux-aarch64: > > + test-luajit: > > runs-on: graviton > > strategy: > > fail-fast: false > > @@ -40,7 +40,7 @@ jobs: > > CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON > > - BUILDTYPE: Release > > CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo > > - name: Linux/aarch64 ${{ matrix.BUILDTYPE }} GC64:ON > > + name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON > > steps: > > - uses: actions/checkout@v2.3.4 > > with: > > @@ -56,3 +56,22 @@ jobs: > > run: cmake --build . --parallel $(($(nproc) + 1)) > > - name: test > > run: cmake --build . --parallel $(($(nproc) + 1)) --target test > > + > > + test-tarantool-debug-w-GC64: > > + name: Tarantool Debug GC64:ON > > + needs: test-luajit > > + uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master > > I suggest to set one variable for this to reduce copy-paste. > Also, why do you prefer copy workflows instead matrix usage? > The only thing changes here -- buildtype. Unfortunately, reusable workflow can't use test matrices, so we have no other option than copy-paste this several times. I might be doing something wrong, but I've failed with this exercise. You can try by yourself, but now you're warned. > > > + with: > > + GC64: ON > > + buildtype: Debug > > + host: graviton > > + revision: ${{ github.sha }} > > + test-tarantool-release-w-GC64: > > + name: Tarantool Release GC64:ON > > + needs: test-luajit > > + uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master > > + with: > > + GC64: ON > > + buildtype: RelWithDebInfo > > + host: graviton > > + revision: ${{ github.sha }} > > diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml > > index d6434afe..3eaf436d 100644 > > --- a/.github/workflows/linux-x86_64.yml > > +++ b/.github/workflows/linux-x86_64.yml > > @@ -1,4 +1,4 @@ > > -name: "LuaJIT test workflow (Linux/x86_64)" > > +name: "Linux/x86_64 test workflow" > > > > on: > > push: > > @@ -29,7 +29,7 @@ concurrency: > > cancel-in-progress: true > > > > jobs: > > - test-linux-x86_64: > > + test-luajit: > > runs-on: ubuntu-20.04-self-hosted > > strategy: > > fail-fast: false > > @@ -41,7 +41,7 @@ jobs: > > CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON > > - BUILDTYPE: Release > > CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo > > - name: Linux/x86_64 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }} > > + name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }} > > steps: > > - uses: actions/checkout@v2.3.4 > > with: > > @@ -57,3 +57,40 @@ jobs: > > run: cmake --build . --parallel $(($(nproc) + 1)) > > - name: test > > run: cmake --build . --parallel $(($(nproc) + 1)) --target test > > + > > + test-tarantool-debug-wo-GC64: > > + name: Tarantool Debug GC64:OFF > > + needs: test-luajit > > + uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master > > + with: > > + GC64: OFF > > Same question here, but also with GC64 mode. Ditto. > > > + buildtype: Debug > > + host: ubuntu-20.04-self-hosted > > + revision: ${{ github.sha }} > > + test-tarantool-debug-w-GC64: > > + name: Tarantool Debug GC64:ON > > + needs: test-luajit > > + uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master > > + with: > > + GC64: ON > > + buildtype: Debug > > + host: ubuntu-20.04-self-hosted > > + revision: ${{ github.sha }} > > + test-tarantool-release-wo-GC64: > > + name: Tarantool Release GC64:OFF > > + needs: test-luajit > > + uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master > > + with: > > + GC64: OFF > > + buildtype: RelWithDebInfo > > + host: ubuntu-20.04-self-hosted > > + revision: ${{ github.sha }} > > + test-tarantool-release-w-GC64: > > + name: Tarantool Release GC64:ON > > + needs: test-luajit > > + uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master > > + with: > > + GC64: ON > > + buildtype: RelWithDebInfo > > + host: ubuntu-20.04-self-hosted > > + revision: ${{ github.sha }} > > -- > > 2.34.0 > > > > -- > Best regards, > Sergey Kaplun -- Best regards, IM