From patchwork Mon Jun 28 21:15:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: M X-Patchwork-Id: 30766 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 06FEA27BC78; Mon, 28 Jun 2021 22:17:12 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, FREEMAIL_FROM,MAILING_LIST_MULTI,SPF_HELO_PASS,T_DKIM_INVALID, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id DAE1D27BC81 for ; Mon, 28 Jun 2021 22:17:10 +0100 (BST) Received: from localhost ([::1]:54680 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lxycv-0002Gc-T5 for patchwork@mira.cbaines.net; Mon, 28 Jun 2021 17:17:09 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:41080) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lxyco-0002DT-6u for guix-patches@gnu.org; Mon, 28 Jun 2021 17:17:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:41624) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lxycn-0006W1-SJ for guix-patches@gnu.org; Mon, 28 Jun 2021 17:17:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lxycn-00073k-OJ for guix-patches@gnu.org; Mon, 28 Jun 2021 17:17:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#48320] [PATCH v2] lint: Verify if #:tests? is respected in the 'check' phase. Resent-From: Maxime Devos Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 28 Jun 2021 21:17:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 48320 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 48320@debbugs.gnu.org Cc: Mathieu Othacehe Received: via spool by 48320-submit@debbugs.gnu.org id=B48320.162491500527111 (code B ref 48320); Mon, 28 Jun 2021 21:17:01 +0000 Received: (at 48320) by debbugs.gnu.org; 28 Jun 2021 21:16:45 +0000 Received: from localhost ([127.0.0.1]:53170 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lxycR-000737-TV for submit@debbugs.gnu.org; Mon, 28 Jun 2021 17:16:45 -0400 Received: from baptiste.telenet-ops.be ([195.130.132.51]:60170) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lxycP-00072v-EI for 48320@debbugs.gnu.org; Mon, 28 Jun 2021 17:16:38 -0400 Received: from ptr-bvsjgyjmffd7q9timvx.18120a2.ip6.access.telenet.be ([IPv6:2a02:1811:8c09:9d00:aaf1:9810:a0b8:a55d]) by baptiste.telenet-ops.be with bizsmtp id NlGa2500R0mfAB401lGbsF; Mon, 28 Jun 2021 23:16:35 +0200 Message-ID: <44f0c8b823b0f6f8e5388ff6c1d90e76fa09bf2c.camel@telenet.be> From: Maxime Devos Date: Mon, 28 Jun 2021 23:15:42 +0200 In-Reply-To: <2b0fee1845a66e1fb126b4bbf1c9892b7c648a3a.camel@telenet.be> References: <2b0fee1845a66e1fb126b4bbf1c9892b7c648a3a.camel@telenet.be> User-Agent: Evolution 3.34.2 MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telenet.be; s=r21; t=1624914995; bh=iCsRZOeOCnZ1tmtzL3I8gXlEPb5FPfAl3F+Yn1sFO50=; h=Subject:From:To:Cc:Date:In-Reply-To:References; b=FnB5t9s1vSN1Vso5aGjZxs/foE+pk2xUkNFtsYJ+1msuLi7EXSi62bZKmWLrtHhuO oQJjtzaMYk4zGelAvENzNzmn98bIz08pep/G3D6hqtk2XLfKI0hYLafSTGQnJY3jq9 pRng5GrnHUbHN280v4wCKOAO5L5pDzPWQQH25YxYhM7uL6sfewSGF62bZtNd9XmzN8 TUmIR4FDsiIkvHDxca3Z1TwQZCyDMRL5Yso0q2zjDc1ZukDdvPlAiECpgobXC2Whh7 Hj9AFrm8hYv2eYgEgn97z03GSs/Pe0oUXEpd4vUwFllBxOgRZsjMxoyQy2/MMTIxKI MHkmvhZFiUxtA== X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches Hi Guix, This is a v2. It detects some more cases (e.g. python-dateutil dejagnu and eigen). It also allows letting '#:phases' be a G-exp. With thanks to Mathieu Othacehe. Greetings, Maxime. From 604cd00c3fcce436d23f05ff7496a6ea1200594e Mon Sep 17 00:00:00 2001 From: Maxime Devos Date: Mon, 28 Jun 2021 20:44:16 +0200 Subject: [PATCH v2 2/2] lint: Verify if #:tests? is respected in the 'check' phase. To: 48320@debbugs.gnu.org Cc: Mathieu Othacehe There have been a few patches to the mailing list lately not respecting this, and this linter detects 368 package definitions that could be modified to support the --without-tests package transformation. * guix/lint.scm (check-optional-tests): New linter. (%local-checkers)[optional-tests]: Add it. * tests/lint.scm (package-with-phase-changes): New procedure. ("optional-tests: no check phase") ("optional-tests: check hase respects #:tests?") ("optional-tests: check phase ignores #:tests?") ("optional-tests: do not crash when #:phases is invalid") ("optional-tests: allow G-exps (no warning)") ("optional-tests: allow G-exps (warning)") ("optional-tests: complicated 'check' phase"): New tests. --- guix/lint.scm | 60 ++++++++++++++++++++++++++++++++++++++++++++- tests/lint.scm | 66 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/guix/lint.scm b/guix/lint.scm index d65d5ce8f9..7fdc330306 100644 --- a/guix/lint.scm +++ b/guix/lint.scm @@ -40,7 +40,8 @@ #:use-module (guix packages) #:use-module (guix i18n) #:use-module ((guix gexp) - #:select (local-file? local-file-absolute-file-name)) + #:select (gexp? local-file? local-file-absolute-file-name + gexp->approximate-sexp)) #:use-module (guix licenses) #:use-module (guix records) #:use-module (guix grafts) @@ -88,6 +89,7 @@ check-source check-source-file-name check-source-unstable-tarball + check-optional-tests check-mirror-url check-github-url check-license @@ -1050,6 +1052,58 @@ descriptions maintained upstream." (define exception-with-kind-and-args? (exception-predicate &exception-with-kind-and-args)) +(define (check-optional-tests package) + "Emit a warning if the test suite is run unconditionally." + (define (sexp-uses-tests?? sexp) + "Test if SEXP contains the symbol 'tests?'." + (sexp-contains-atom? sexp 'tests?)) + (define (sexp-contains-atom? sexp atom) + "Test if SEXP contains ATOM." + (if (pair? sexp) + (or (sexp-contains-atom? (car sexp) atom) + (sexp-contains-atom? (cdr sexp) atom)) + (eq? sexp atom))) + (define (check-check-procedure expression) + (match expression + (`(,(or 'let 'let*) . ,_) + (check-check-procedure (car (last-pair expression)))) + (`(,(or 'lambda 'lambda*) ,_ . ,code) + (if (sexp-uses-tests?? code) + '() + (list (make-warning package + ;; TRANSLATORS: check and #:tests? are a + ;; Scheme symbol and keyword respectively + ;; and should not be translated. + (G_ "the 'check' phase should respect #:tests?") + #:field 'arguments)))) + (_ '()))) + (define (check-phases-delta delta) + (match delta + (`(replace 'check ,expression) + (check-check-procedure expression)) + (_ '()))) + (define (check-phases-deltas deltas) + (match deltas + (() '()) + ((head . tail) + (or (check-phases-delta head) + (check-phases-deltas tail))) + (_ (list (make-warning package + ;; TRANSLATORS: modify-phases is a Scheme + ;; syntax and must not be translated. + (G_ "incorrect call to ‘modify-phases’") + #:field 'arguments))))) + (apply (lambda* (#:key phases #:allow-other-keys) + (define phases/sexp + (if (gexp? phases) + (gexp->approximate-sexp phases) + phases)) + (match phases/sexp + (`(modify-phases ,_ . ,changes) + (check-phases-deltas changes)) + (_ '()))) + (package-arguments package))) + (define* (check-derivation package #:key store) "Emit a warning if we fail to compile PACKAGE to a derivation." (define (try store system) @@ -1590,6 +1644,10 @@ them for PACKAGE." (description "Make sure the 'license' field is a \ or a list thereof") (check check-license)) + (lint-checker + (name 'optional-tests) + (description "Make sure tests are only run when requested") + (check check-optional-tests)) (lint-checker (name 'mirror-url) (description "Suggest 'mirror://' URLs") diff --git a/tests/lint.scm b/tests/lint.scm index fae346e724..33705f7cd3 100644 --- a/tests/lint.scm +++ b/tests/lint.scm @@ -9,6 +9,7 @@ ;;; Copyright © 2018, 2019 Arun Isaac ;;; Copyright © 2020 Timothy Sample ;;; Copyright © 2021 Xinglu Chen +;;; Copyright © 2021 Maxime Devos ;;; ;;; This file is part of GNU Guix. ;;; @@ -38,7 +39,7 @@ #:use-module (guix lint) #:use-module (guix ui) #:use-module (guix swh) - #:use-module ((guix gexp) #:select (local-file)) + #:use-module ((guix gexp) #:select (gexp local-file gexp?)) #:use-module ((guix utils) #:select (call-with-temporary-directory)) #:use-module ((guix import hackage) #:select (%hackage-url)) #:use-module ((guix import stackage) #:select (%stackage-url)) @@ -744,6 +745,69 @@ (sha256 %null-sha256)))))) (check-source-unstable-tarball pkg))) +(define (package-with-phase-changes changes) + (dummy-package "x" + (arguments `(#:phases + ,(if (gexp? changes) + #~(modify-phases %standard-phases + #$@changes) + `(modify-phases %standard-phases + ,@changes)))))) + +(test-equal "optional-tests: no check phase" + '() + (let ((pkg (package-with-phase-changes '()))) + (check-optional-tests pkg))) + +(test-equal "optional-tests: check phase respects #:tests?" + '() + (let ((pkg (package-with-phase-changes + '((replace 'check + (lambda* (#:key tests? #:allow-other-keys?) + (when tests? + (invoke "./the-test-suite")))))))) + (check-optional-tests pkg))) + +(test-equal "optional-tests: check phase ignores #:tests?" + "the 'check' phase should respect #:tests?" + (let ((pkg (package-with-phase-changes + '((replace 'check + (lambda _ + (invoke "./the-test-suite"))))))) + (single-lint-warning-message + (check-optional-tests pkg)))) + +(test-equal "optional-tests: do not crash when #:phases is invalid" + "incorrect call to ‘modify-phases’" + (let ((pkg (package-with-phase-changes 'this-is-not-a-list))) + (single-lint-warning-message + (check-optional-tests pkg)))) + +(test-equal "optional-tests: allow G-exps (no warning)" + '() + (let ((pkg (package-with-phase-changes #~()))) + (check-optional-tests pkg))) + +(test-equal "optional-tests: allow G-exps (warning)" + "the 'check' phase should respect #:tests?" + (let ((pkg (package-with-phase-changes + #~((replace 'check + (lambda _ + (invoke "/the-test-suite"))))))) + (single-lint-warning-message + (check-optional-tests pkg)))) + +(test-equal "optional-tests: complicated 'check' phase" + "the 'check' phase should respect #:tests?" + (let ((pkg (package-with-phase-changes + '((replace 'check + (lambda* (#:key inputs tests? #:allow-other-keys) + (let ((something (stuff from inputs or native-inputs))) + (delete-file "dateutil/test/test_utils.py") + (invoke "pytest" "-vv")))))))) + (single-lint-warning-message + (check-optional-tests pkg)))) + (test-equal "source: 200" '() (with-http-server `((200 ,%long-string)) -- 2.32.0