From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id SL/3KD5TbGJOHgAAbAwnHQ (envelope-from ) for ; Fri, 29 Apr 2022 23:06:06 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id qFMwKT5TbGJO6wAAauVa8A (envelope-from ) for ; Fri, 29 Apr 2022 23:06:06 +0200 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 435AA3E060 for ; Fri, 29 Apr 2022 23:06:06 +0200 (CEST) Received: from localhost ([::1]:47678 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nkXoT-0007LG-AR for larch@yhetil.org; Fri, 29 Apr 2022 17:06:05 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53530) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nkXoQ-0007Fp-0F for gwl-devel@gnu.org; Fri, 29 Apr 2022 17:06:02 -0400 Received: from smtp.polymtl.ca ([132.207.4.11]:54524) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nkXoM-0006A4-OE for gwl-devel@gnu.org; Fri, 29 Apr 2022 17:06:00 -0400 Received: from localhost (modemcable094.169-200-24.mc.videotron.ca [24.200.169.94]) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 23TL5oKs030382 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 29 Apr 2022 17:05:55 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 23TL5oKs030382 To: Ricardo Wurmus Cc: gwl-devel@gnu.org Subject: Re: [PATCH] tests/examples: Add running of workflow examples In-Reply-To: <87sfpvzkmj.fsf@elephly.net> References: <20220429175500.17176-1-olivier.dion@polymtl.ca> <87sfpvzkmj.fsf@elephly.net> Date: Fri, 29 Apr 2022 17:05:50 -0400 Message-ID: <87sfpva9ch.fsf@laura> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Poly-FromMTA: (modemcable094.169-200-24.mc.videotron.ca [24.200.169.94]) at Fri, 29 Apr 2022 21:05:50 +0000 Received-SPF: pass client-ip=132.207.4.11; envelope-from=olivier.dion@polymtl.ca; helo=smtp.polymtl.ca X-Spam_score_int: -41 X-Spam_score: -4.2 X-Spam_bar: ---- X-Spam_report: (-4.2 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: gwl-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gwl-devel-bounces+larch=yhetil.org@gnu.org Sender: "gwl-devel" Reply-to: Olivier Dion From: Olivier Dion via X-Migadu-Flow: FLOW_IN X-Migadu-To: larch@yhetil.org X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1651266366; h=from:from:sender:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=klKIDR306RlLq0mCuJW2YAXu/uF3IMSnb4I5/5DTivY=; b=U7u0Joi/4NiNaiHESZG0mxHitTcYO0XkBvsWrxCGbcpxNH+bTJcF9rnBFsIednhkdFLbjn T+L65/ppqRy9BMYGYFtVcavFyc+rq/EWs8G9+q6qdVSJCmM6rwOZ3RaJWmacPG8gnZvohK d0JP27vdndM4sBOEEvayRgsGmVNKt9lQlVTvUOiM6FEjlNW8VvzpyVyKKvEkh2s0ulrVl1 c3F3lkk3vZHVgDvaerLV8/uvztEtc5BOlMcMG+XpBTQ12kGXsi8XpInrT0kDOj+wokp7IK B0cy7FGU1rSn/klW48Oe5sziIVfeSAROVE/Z2uN69OHWIY5eyJcEZcWU2Jax8g== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1651266366; a=rsa-sha256; cv=none; b=nZc+sl34RV3eAQ8pLPLwOZxMLycaVlQ/WaKU+Or3t5cWFF7Rf2zd00FSZ5RRQx3mfgqd/J 30vnXH5ixvoYlMTPpEg5A6C6HBdlTsk1bGUCXCUTGxlce9p8KPKisfFxUazZqKfFlxNtWx WBCmk9bpVmNM4WSBe+euDKsmvClbQ8I8H4SbO83IzBfyu2I7cmWvubYlRzW7D+2gysgEpM lIcGqOX1YLP4r+ih38wiQPFrYAPmLWS+C/tHOTK3XNl7k2pUzteZBh/q5HEnevzGXppWaz 0CW9C8Y2V8Vt49OI04fn0VHbxfP24u4uoyOerp7GXNEaCTEFfpjqzhjo2BmxTQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "gwl-devel-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="gwl-devel-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: -3.00 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "gwl-devel-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="gwl-devel-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 435AA3E060 X-Spam-Score: -3.00 X-Migadu-Scanner: scn1.migadu.com X-TUID: iTHAGxfWrlKj On Fri, 29 Apr 2022, Ricardo Wurmus wrote: > Hi Olivier, > > thanks for the patch! > >> End to end testing of pre-defined scenarios are a good way to check for >> regression. >> >> Here we introduce testing of some examples available in the documentatio= n. That >> way, we're sure that new users should be able to run them without proble= ms. >> >> Each scenario is a different test and is run in a different temporary >> directory which get destroyed if the scenario succeeded. > > It=E2=80=99s a good idea to run the examples. However, this requires a f= ully > functional Guix installation (which we don=E2=80=99t have when building w= ith > Guix, for example), so we should not include them in =E2=80=9Cmake check= =E2=80=9D but > add a separate target for these tests. An alternative may be to do what > Guix does for system tests, but I=E2=80=99d be okay with just having a se= parate > target that is run manually before releases or in CI. > >> +(define-syntax-rule (with-directory-excursion dir body body* ...) >> + (let ((old-dir (getcwd))) >> + (dynamic-wind >> + (lambda () (chdir dir)) >> + (lambda () body body* ...) >> + (lambda () (chdir old-dir))))) >> + >> +(with-directory-excursion >> + (string-append top-srcdir "/doc/examples") >> + >> + (define (process-success? status) >> + (=3D 0 (or (status:exit-val status) >> + (status:term-sig status)))) > > Use ZERO? here. I learn everyday with you. I did not know about it ^^. >> + (define scenarios >> + (list "extended-example-workflow.w")) > > Should these better be discovered automatically via SCANDIR? Well some example are not full workflow. Only processes. So I think that yes, if we set a common prefix for full workflow: (scandir "." (cut string-prefix "exemple-workflow" <>)) However, it's nice -- even more true when debugging -- to be able to cherry-pick the tests. This is easier with the list above by commenting tests that are passing. >> + (for-each (lambda (example) >> + (test-assert example >> + (let* ((tmp-dir (mkdtemp >> + (format #f "gwl-example-~a.XXXXXX" exa= mple))) >> + (abs-example (canonicalize-path example)) >> + (success? >> + (with-directory-excursion tmp-dir >> + (process-success? >> + (system >> + (format #f "guix workflow run -fc ~a -l all" >> + abs-example)))))) > > Please don=E2=80=99t use SYSTEM. How about > > (system* "guix" "workflow" "run" "--force" "--container" > "--log-events=3Dall" (canonicalize-path example)) Yes I like it better too thanks. >> + (if success? >> + (system* "rm" "-rf" tmp-dir) > > Why shell out to RM when we have DELETE-FILE and its recursive friend in > Guix? I=E2=80=99d also rather move clean-up work to a DYNAMIC-UNWIND han= dler. I hesitated to include Guix's module in test. If you're okay with it, I will use the helpers available in Guix. I concur that the cleanup should be in dynamic-unwind. >> + (format (error-output-port) >> + "Example directory: ~a\n" tmp-dir)) > > Nitpick: ~% instead of \n. Is there a reason why? I don't mind I just never really understood why scheme has this special format rule for newline. >> + success?))) >> + scenarios)) > > This FOR-EACH loop combines test definition with test running, which > seems wrong to me. Maybe SRFI-64 is not the best fit for tests that > only care about whether a shell command was run successfully. Perhaps > we should do as Guix does and just have a shell script to run these > tests. What do you find wrong about it? We could re-write it as: (define (run-example path) ...) (test-assert (run-example "extended-workflow.w")) (test-assert (run-example "...")) if you like it better. However, we lose the power of SCANDIR mentioned above. Let me know, I will send a v2 with your above recommendations and answers to my questions. --=20 Olivier Dion oldiob.dev