[PR PATCH] Add make_check var to control when tests are run

[PR PATCH] Add make_check var to control when tests are run

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 1609 bytes --]

There is a new pull request by ericonr against master on the void-packages repository

https://github.com/ericonr/void-packages checks
https://github.com/void-linux/void-packages/pull/28312

Add make_check var to control when tests are run
<!-- Mark items with [x] where applicable -->

#### General
- [ ] This is a new package and it conforms to the [quality requirements](https://github.com/void-linux/void-packages/blob/master/Manual.md#quality-requirements)

#### Have the results of the proposed changes been tested?
- [ ] I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
- [ ] I generally don't use the affected packages but briefly tested this PR

<!--
If GitHub CI cannot be used to validate the build result (for example, if the
build is likely to take several hours), make sure to
[skip CI](https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#continuous-integration).
When skipping CI, uncomment and fill out the following section.
Note: for builds that are likely to complete in less than 2 hours, it is not
acceptable to skip CI.
-->
<!-- 
#### Does it build and run successfully? 
(Please choose at least one native build and, if supported, at least one cross build. More are better.)
- [ ] I built this PR locally for my native architecture, (ARCH-LIBC)
- [ ] I built this PR locally for these architectures (if supported. mark crossbuilds):
  - [ ] aarch64-musl
  - [ ] armv7l
  - [ ] armv6l-musl
-->


A patch file from https://github.com/void-linux/void-packages/pull/28312.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-checks-28312.patch --]
[-- Type: text/x-diff, Size: 3009 bytes --]

From 46a16e0a8fe4fe4f321fa18eeb7f241645527b5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Fri, 29 Jan 2021 21:47:18 -0300
Subject: [PATCH 1/2] xbps-src: add -K option to getopt string.

Was forgotten when the actual option was added.
---
 xbps-src | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xbps-src b/xbps-src
index 77969af84a1..c9219386916 100755
--- a/xbps-src
+++ b/xbps-src
@@ -354,7 +354,7 @@ readonly XBPS_SRC_VERSION="113"
 export XBPS_MACHINE=$(xbps-uhelper -C /dev/null arch)
 
 XBPS_OPTIONS=
-XBPS_OPTSTRING="1a:c:CEfgGhH:iIj:Lm:No:p:qQr:tV"
+XBPS_OPTSTRING="1a:c:CEfgGhH:iIj:Lm:No:p:qQKr:tV"
 
 # Preprocess arguments in order to allow options before and after XBPS_TARGET.
 eval set -- $(getopt "$XBPS_OPTSTRING" "$@");

From ff30aea9a6d2d421739febe053a79e95b523fab3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Fri, 29 Jan 2021 21:47:59 -0300
Subject: [PATCH 2/2] xbps-src/libexec: add more make_check option.

Allows a template to define in which circumstances its do_check phase
should run, without requiring custom do_check definitions in each
template (also makes it easier to change things in how build styles do
things without checking as many templates).

Add to Manual as well.
---
 Manual.md                                   |  5 +++++
 common/xbps-src/libexec/xbps-src-docheck.sh | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/Manual.md b/Manual.md
index ed73ec338fa..e7c1f28db67 100644
--- a/Manual.md
+++ b/Manual.md
@@ -589,6 +589,11 @@ patches to the package sources during `do_patch()`. Patches are stored in
 - `disable_parallel_build` If set the package won't be built in parallel
 and `XBPS_MAKEJOBS` has no effect.
 
+- `make_check` Sets the cases in which the `check` phase is run. Can be `yes` (the default) to run if
+`XBPS_CHECK_PKGS` is set, `extended` to run if `XBPS_CHECK_PKGS` is `full` and `no` to never run.
+This option should usually be accompanied by a comment explaining why it was set, especially when
+set to `no`.
+
 - `keep_libtool_archives` If enabled the `GNU Libtool` archives won't be removed. By default those
 files are always removed automatically.
 
diff --git a/common/xbps-src/libexec/xbps-src-docheck.sh b/common/xbps-src/libexec/xbps-src-docheck.sh
index 6be6c9d6ead..1cb9f6ab13a 100755
--- a/common/xbps-src/libexec/xbps-src-docheck.sh
+++ b/common/xbps-src/libexec/xbps-src-docheck.sh
@@ -32,6 +32,17 @@ if [ -z "$XBPS_CHECK_PKGS" ]; then
     exit 0
 fi
 
+if [ "$make_check" = no ]; then
+    msg_normal "${pkgname}-${version}_${revision}: skipping check (make_check=no) ...\n"
+    exit 0
+fi
+
+if [ "$make_check" = extended -a "$XBPS_CHECK_PKGS" != full ]; then
+    msg_normal \
+        "${pkgname}-${version}_${revision}: skipping check (make_check=extended and XBPS_CHECK_PKGS is not 'full') ...\n"
+    exit 0
+fi
+
 for f in $XBPS_COMMONDIR/environment/check/*.sh; do
     source_file "$f"
 done

Re: Add make_check var to control when tests are run

[Chocimier]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

New comment by Chocimier on void-packages repository

https://github.com/void-linux/void-packages/pull/28312#issuecomment-770210989

Comment:
Looks good.
I think now manual need a sectin explainig all that complexity, something along

    If default do_check do nothing, define one to run upstream tests, if any.
    If tests run over dozen of minutes, put `if XBPS_CHECK_PKGS = full` in do_check, or else make_check

Re: Add make_check var to control when tests are run

[Chocimier]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

New comment by Chocimier on void-packages repository

https://github.com/void-linux/void-packages/pull/28312#issuecomment-770210989

Comment:
Looks good.
I think now manual need a sectin explainig all that complexity, something along

    If default do_check do nothing, define one to run upstream tests, if any.
    If tests run over dozen of minutes, put `if XBPS_CHECK_PKGS = full` in do_check, or else make_check

Re: Add make_check var to control when tests are run

[Chocimier]

[-- Attachment #1: Type: text/plain, Size: 486 bytes --]

New comment by Chocimier on void-packages repository

https://github.com/void-linux/void-packages/pull/28312#issuecomment-770211177

Comment:
Looks good.
I think now manual need a section explainig all that complexity, something along

    If default do_check do nothing, define one to run upstream tests, if any.
    If tests run over dozen of minutes, put `if XBPS_CHECK_PKGS = full` in do_check, or else make_check=extended.
    Then XBPS_CHECK_PKGS controls what tests to run.

Re: [PR PATCH] [Updated] Add make_check var to control when tests are run

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]

There is an updated pull request by ericonr against master on the void-packages repository

https://github.com/ericonr/void-packages checks
https://github.com/void-linux/void-packages/pull/28312

Add make_check var to control when tests are run
<!-- Mark items with [x] where applicable -->

#### General
- [ ] This is a new package and it conforms to the [quality requirements](https://github.com/void-linux/void-packages/blob/master/Manual.md#quality-requirements)

#### Have the results of the proposed changes been tested?
- [ ] I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
- [ ] I generally don't use the affected packages but briefly tested this PR

<!--
If GitHub CI cannot be used to validate the build result (for example, if the
build is likely to take several hours), make sure to
[skip CI](https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#continuous-integration).
When skipping CI, uncomment and fill out the following section.
Note: for builds that are likely to complete in less than 2 hours, it is not
acceptable to skip CI.
-->
<!-- 
#### Does it build and run successfully? 
(Please choose at least one native build and, if supported, at least one cross build. More are better.)
- [ ] I built this PR locally for my native architecture, (ARCH-LIBC)
- [ ] I built this PR locally for these architectures (if supported. mark crossbuilds):
  - [ ] aarch64-musl
  - [ ] armv7l
  - [ ] armv6l-musl
-->


A patch file from https://github.com/void-linux/void-packages/pull/28312.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-checks-28312.patch --]
[-- Type: text/x-diff, Size: 6020 bytes --]

From 46a16e0a8fe4fe4f321fa18eeb7f241645527b5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Fri, 29 Jan 2021 21:47:18 -0300
Subject: [PATCH 1/2] xbps-src: add -K option to getopt string.

Was forgotten when the actual option was added.
---
 xbps-src | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xbps-src b/xbps-src
index 77969af84a1..c9219386916 100755
--- a/xbps-src
+++ b/xbps-src
@@ -354,7 +354,7 @@ readonly XBPS_SRC_VERSION="113"
 export XBPS_MACHINE=$(xbps-uhelper -C /dev/null arch)
 
 XBPS_OPTIONS=
-XBPS_OPTSTRING="1a:c:CEfgGhH:iIj:Lm:No:p:qQr:tV"
+XBPS_OPTSTRING="1a:c:CEfgGhH:iIj:Lm:No:p:qQKr:tV"
 
 # Preprocess arguments in order to allow options before and after XBPS_TARGET.
 eval set -- $(getopt "$XBPS_OPTSTRING" "$@");

From 4dd35867731c0227409409ba9bfc89232a68a559 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Fri, 29 Jan 2021 21:47:59 -0300
Subject: [PATCH 2/2] xbps-src/libexec: add make_check option.

Allows a template to define in which circumstances its do_check phase
should run, without requiring custom do_check definitions in each
template (also makes it easier to change how build styles do things
without checking as many templates).

Add to Manual and CONTRIBUTING as well.
---
 CONTRIBUTING.md                             |  6 +++++-
 Manual.md                                   | 12 +++++++++++-
 common/xbps-src/libexec/xbps-src-docheck.sh | 11 +++++++++++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index cd3d6aad5dd..d6a7299f64b 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -87,11 +87,15 @@ When you make changes to your pull request, please *do not close and reopen your
 
 #### Continuous Integration
 
-Pull requests are automatically submitted for Continuous Integration (CI) testing to ensure packages build on various combinations of libc and architecture.
+Pull requests are automatically submitted for Continuous Integration (CI) testing to ensure packages build and pass their tests (on native builds) on various combinations of C library and architecture.
 Packages that take longer than 120 minutes or need more than 14G of storage to complete their build (for example, Firefox or the Linux kernel) will fail CI and should include `[ci skip]` in the PR title or body (the comment field when the PR is being opened) to avoid wasting CI builder time.
 Use your best judgment on build times based on your local building experience. If you skip CI when submitting a PR, please build and cross-build for a variety of architectures locally, with both glibc and musl, and note your local results in PR comments.
 Make sure to cover 64-bit and 32-bit architectures.
 
+If you notice a failure in CI that didn't happen locally, that is likely because you didn't run tests locally.
+Use `./xbps-src -Q pkg <package>` to do so.
+Some tests won't work in the CI environment or at all, and their templates should encode this information using the `make_check` variable.
+
 Continuous Integration will also check if the templates you have changed
 comply with the our guidelines. At the moment not all packages comply with the rules, so if you update a package, it may report errors about places you haven't touched. Please feel free to fix those errors too.
 
diff --git a/Manual.md b/Manual.md
index ed73ec338fa..acd23a32b42 100644
--- a/Manual.md
+++ b/Manual.md
@@ -166,7 +166,12 @@ can be used to perform other operations before configuring the package.
 
 - `build` This phase compiles/prepares the `source files` via `make` or any other compatible method.
 
-- `check` This optional phase checks the result of the `build` phase for example by running `make -k check`.
+- `check` This optional phase checks the result of the `build` phase by running the testsuite provided by the package.
+If the default `do_check` function provided by the build style doesn't do anything, the template should set
+`make_check_target` and/or `make_check_args` appropriately or define its own `do_check` function. If tests take too long
+or can't run in all environments, they should be run only if `XBPS_CHECK_PKGS` is `full`, which means they should either
+be under a `[ "$XBPS_CHECK_PKGS" = full ]` conditional (especially useful with custom `do_check`) or `make_check=extended`
+should be set in the template.
 
 - `install` This phase installs the `package files` into the package destdir `<masterdir>/destdir/<pkgname>-<version>`,
 via `make install` or any other compatible method.
@@ -589,6 +594,11 @@ patches to the package sources during `do_patch()`. Patches are stored in
 - `disable_parallel_build` If set the package won't be built in parallel
 and `XBPS_MAKEJOBS` has no effect.
 
+- `make_check` Sets the cases in which the `check` phase is run. Can be `yes` (the default) to run if
+`XBPS_CHECK_PKGS` is set, `extended` to run if `XBPS_CHECK_PKGS` is `full` and `no` to never run.
+This option should usually be accompanied by a comment explaining why it was set, especially when
+set to `no`.
+
 - `keep_libtool_archives` If enabled the `GNU Libtool` archives won't be removed. By default those
 files are always removed automatically.
 
diff --git a/common/xbps-src/libexec/xbps-src-docheck.sh b/common/xbps-src/libexec/xbps-src-docheck.sh
index 6be6c9d6ead..1cb9f6ab13a 100755
--- a/common/xbps-src/libexec/xbps-src-docheck.sh
+++ b/common/xbps-src/libexec/xbps-src-docheck.sh
@@ -32,6 +32,17 @@ if [ -z "$XBPS_CHECK_PKGS" ]; then
     exit 0
 fi
 
+if [ "$make_check" = no ]; then
+    msg_normal "${pkgname}-${version}_${revision}: skipping check (make_check=no) ...\n"
+    exit 0
+fi
+
+if [ "$make_check" = extended -a "$XBPS_CHECK_PKGS" != full ]; then
+    msg_normal \
+        "${pkgname}-${version}_${revision}: skipping check (make_check=extended and XBPS_CHECK_PKGS is not 'full') ...\n"
+    exit 0
+fi
+
 for f in $XBPS_COMMONDIR/environment/check/*.sh; do
     source_file "$f"
 done

Re: [PR PATCH] [Merged]: Add make_check var to control when tests are run

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]

There's a merged pull request on the void-packages repository

Add make_check var to control when tests are run
https://github.com/void-linux/void-packages/pull/28312

Description:
<!-- Mark items with [x] where applicable -->

#### General
- [ ] This is a new package and it conforms to the [quality requirements](https://github.com/void-linux/void-packages/blob/master/Manual.md#quality-requirements)

#### Have the results of the proposed changes been tested?
- [ ] I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
- [ ] I generally don't use the affected packages but briefly tested this PR

<!--
If GitHub CI cannot be used to validate the build result (for example, if the
build is likely to take several hours), make sure to
[skip CI](https://github.com/void-linux/void-packages/blob/master/CONTRIBUTING.md#continuous-integration).
When skipping CI, uncomment and fill out the following section.
Note: for builds that are likely to complete in less than 2 hours, it is not
acceptable to skip CI.
-->
<!-- 
#### Does it build and run successfully? 
(Please choose at least one native build and, if supported, at least one cross build. More are better.)
- [ ] I built this PR locally for my native architecture, (ARCH-LIBC)
- [ ] I built this PR locally for these architectures (if supported. mark crossbuilds):
  - [ ] aarch64-musl
  - [ ] armv7l
  - [ ] armv6l-musl
-->