Add initial GitHub Actions workflow #374

zmughal · 2023-03-30T22:51:48Z

Since Travis CI is changed its terms, many FOSS projects on GitHub have started
using GitHub Actions instead. This was previously discussed at
#224 (comment).

This PR is an initial working CI workflow to get started with GitHub Actions. I
believe that further work can be done to enable Coveralls code coverage.

~~I can either add that in this PR or in subsequent PRs.~~

I have added Coveralls support to this PR

zmughal · 2023-03-30T23:03:53Z

To make the most out of the Coveralls service for code coverage, enabling the Status API would be helpful as discussed at #224 (comment).

zmughal · 2023-03-31T00:05:54Z

As an aside, if a workflow must be created for the rest of the repos, it might be worth creating a shared set of GitHub Actions as done at https://github.com/PDLPorters/devops/tree/master/github-actions and https://github.com/Perl-GPU/devops/tree/main/github-actions.

This uses Coveralls for coverage reporting.

zmughal · 2023-04-01T21:27:58Z

@cjfields, @carandraug, reaching out for review of this.

Also, thoughts on standardising the CI across the organisation?

cjfields · 2023-04-14T22:52:59Z

@cjfields, @carandraug, reaching out for review of this.

Also, thoughts on standardising the CI across the organisation?

Awesome work, thanks! If we can come up with a standard, yes it makes sense to standardize for all repos under the bioperl org.

Let's give it a week, if @carandraug or others don't have any objections or comments I'll merge.

hlapp · 2023-04-15T15:04:49Z

.github/workflows/ci.yml

+          - { os: 'ubuntu-latest', perl: "5.20" }
+          - { os: 'ubuntu-latest', perl: "5.30" }
+          - { os: 'ubuntu-latest', perl: "5.32" }
+          - { os: 'ubuntu-latest', perl: "5.36" }


Can you say what motivates the selection of these versions, and needing to have 5 explicitly specified ones?

I believe I was looking at CPAN Testers to find a minimum version that had no failures and then added a couple more versions spaced apart to get a sample (this could be changed to test against every stable version). While Perl is very backwards compatible, sometimes getting an idea of when something like a warning appears can help narrow down where it is coming from.

It should probably add Perl 5.6 as well since that is what the dist explicitly has as the minimum version, but this looks wrong after I scanned the code to find if it used newer features:

$ perlver lib/ > output.log $ grep -vP 'v5\.[56]\.' output.log | grep 'v5' | cut -d'|' -f2 | head -n -2 | tr -d ' ' | xargs -n1 perlver --blame ------------------------------------------------------------ File : lib/BioPerl.pm Line : 3 Char : 1 Rule : _pragma_utf8 Version : 5.008 ------------------------------------------------------------ use utf8; ------------------------------------------------------------ ------------------------------------------------------------ File : lib/Bio/PullParserI.pm Line : 296 Char : 21 Rule : _open_scalar Version : 5.008 ------------------------------------------------------------ open my $fake_fh, "+<", \$thing or $self->throw("Could not open file '$thing': $!"); ------------------------------------------------------------ ------------------------------------------------------------ File : lib/Bio/Root/IO.pm Line : 288 Char : 13 Rule : _binmode_2_arg Version : 5.008 ------------------------------------------------------------ binmode ------------------------------------------------------------ ------------------------------------------------------------ File : lib/Bio/Tree/Tree.pm Line : 393 Char : 5 Rule : _open_scalar Version : 5.008 ------------------------------------------------------------ open my $fh, '>', \$string or $self->throw("Could not write '$string' as file: $!"); ------------------------------------------------------------ ------------------------------------------------------------ File : lib/Bio/Tools/GuessSeqFormat.pm Line : 465 Char : 9 Rule : _open_scalar Version : 5.008 ------------------------------------------------------------ open $fh, '<', \$text or $self->throw("Could not read from string: $!"); ------------------------------------------------------------

So the minimum needs to be bumped to Perl v5.8. I'll open an issue for that.

Having the minimum version would help with finding issues where a change actually requires a newer Perl version.

Issue at #375.

@hlapp @zmughal see my comment on #375. I don't think we should go out of our way to support older versions the Perl community isn't actively maintaining/supporting. As for the number of versions tested, maybe minimal/median/latest?

hlapp · 2023-04-15T15:35:57Z

Super cool, thanks so much @zmughal 🎉

zmughal · 2023-04-18T00:37:10Z

@cjfields, @carandraug, reaching out for review of this.
Also, thoughts on standardising the CI across the organisation?

Awesome work, thanks! If we can come up with a standard, yes it makes sense to standardize for all repos under the bioperl org.

If we create a devops repository, I can start gathering data on dependencies there and start putting together a shared GitHub Action for doing the same as what this PR does within that. I can't since I'm not in the org.

carandraug · 2023-04-18T12:09:58Z

I'm not familiar with github actions. However, why are we using cpanm to install the dependencies? I've packaged all the bioperl development dependencies to Debian years ago so they should have already reached ubuntu, so an apt-get install should suffice and be faster (and avoid the problem of dependencies changing in the mean time).

cjfields · 2023-04-18T14:49:53Z

Awesome work, thanks! If we can come up with a standard, yes it makes sense to standardize for all repos under the bioperl org.

If we create a devops repository, I can start gathering data on dependencies there and start putting together a shared GitHub Action for doing the same as what this PR does within that. I can't since I'm not in the org.

That can be fixed pretty easily :)

cjfields · 2023-04-18T15:02:26Z

I'm not familiar with github actions. However, why are we using cpanm to install the dependencies? I've packaged all the bioperl development dependencies to Debian years ago so they should have already reached ubuntu, so an apt-get install should suffice and be faster (and avoid the problem of dependencies changing in the mean time).

I agree, though maybe we need to test both paths somewhat? I'd think cpanm would be the most common (but possibly brittle) installation path, apt the most stable.

zmughal · 2023-04-18T15:18:27Z

I'm not familiar with github actions. However, why are we using cpanm to install the dependencies?

This GitHub Actions workflow is similar to the previous Travis CI set up and that also used cpanm.

I've packaged all the bioperl development dependencies to Debian years ago so they should have already reached ubuntu, so an apt-get install should suffice and be faster (and avoid the problem of dependencies changing in the mean time).

Installing via a CPAN client is the typical scenario for libraries that will go on CPAN as there isn't a way to pin to a specific version for library code (using a cpanfile or Carton would help if pinning is needed, but version pinning that way is more meant to be used for application code).

It wouldn't be too much work to test that as well to establish a baseline of dependency versions, but it would be a separate workflow (and it would have to use the system Perl so could only test against that Perl version). This would either explicitly list the packages or use apt-file to get them:

$ cpanm -q --showdeps ./BioPerl-1.7.8.tar.gz  | perl -lpe 's,::,/,g; $_ = qq{/usr/share/perl5/$_.pm}' | apt-file search -lFf -
libdata-stag-perl
liberror-perl
libgraph-perl
[...]
libwww-perl
[...]
libyaml-perl

I suppose adding this would help Debian users and possibly catch things for the Debian Med and Debian Perl packaging team. I'm thinking it should be done in a container (stable: debian:bullseye, oldstable: debian:buster). I see that the current Dockerfile uses ubuntu:14.04.

cjfields · 2023-04-18T15:49:55Z

Personally I'd say go with the path that is easiest to maintain and build upon. If it works for bioperl-live (which is by far the most complicated), it should work for any other repos under the bioperl org.

zmughal · 2023-04-19T17:14:15Z

@carandraug, I have a branch that tests using Debian containers as well. It's quite small and should be reusable across the organisation: zmughal-contrib@6751bb8.

commit 6751bb8c2d818af41a8cf56b12ba47e2dd6cb4b2

    Add test using Debian container

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 05b8185da..762b2f04f 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -9,6 +9,8 @@ on:
 env:
   PKG_DEPS_UBUNTU: >-
     libdb-dev
+  PKG_DEPS_DEBIAN: >-
+    libdb-dev
 
 jobs:
   dist:
@@ -122,3 +124,29 @@ jobs:
         run: |
           cpanm --verbose --test-only .
           cover -report Coveralls
+
+  containers:
+    needs: dist
+    runs-on: ubuntu-latest
+    container: ${{ matrix.container }}
+    strategy:
+      fail-fast: false
+      matrix:
+        container: ['debian:bullseye', 'debian:bookworm']
+    steps:
+      - name: Get dist artifact
+        uses: actions/download-artifact@v3
+        with:
+          name: dist
+      - name: Setup system deps (apt)
+        if: ${{ startsWith(matrix.container, 'debian:') }}
+        run: |
+          apt-get -y update && apt-get install -y --no-install-recommends perl cpanminus make apt-file
+          apt-file update
+          apt-get install -y --no-install-recommends \
+            ${{ env.PKG_DEPS_DEBIAN }} \
+            $( cpanm -q --showdeps .  | perl -MConfig -MCwd=abs_path '-M5;@prefixes = map abs_path($_), @Config{qw(vendorlibexp vendorarchexp)}' -lpe 's,~.*$,,; s,::,/,g; $mod = $_; $_ = join qq{\n}, map { qq{$_/${mod}.pm} } @prefixes' | apt-file search -lFf - )
+
+      - name: Run tests
+        run: |
+          cpanm --verbose --test-only .

Also, this revealed a failing test! I'll open an issue for that.

zmughal · 2023-04-24T21:37:08Z

Any other specific changes I should make here? This is just the first step and when consolidated into a shared reusable workflow, it can be modified in a single place for the whole organisation.

cjfields · 2023-04-26T20:49:33Z

I don't think there are any objections to this apart from maybe simple tweaks which can be accomplished later. Thanks for taking this on @zmughal !

This is per discussion at: - <bioperl#374 (comment)>, - <bioperl#375>.

This tests against specific Debian releases using all Perl module dependencies that can be installed via `apt-get`. This is per discussion at <bioperl#374 (comment)>.

zmughal · 2023-04-28T00:00:11Z

I don't think there are any objections to this apart from maybe simple tweaks which can be accomplished later. Thanks for taking this on @zmughal !

Great! Another thing that would be useful to have is to go to https://coveralls.io/github/bioperl/bioperl-live/settings and enable the Status API so that the coverage can be displayed as part of the pull request. A screenshot of an example setting:

cjfields · 2023-04-28T15:12:14Z

I don't think there are any objections to this apart from maybe simple tweaks which can be accomplished later. Thanks for taking this on @zmughal !

Great! Another thing that would be useful to have is to go to https://coveralls.io/github/bioperl/bioperl-live/settings and enable the Status API so that the coverage can be displayed as part of the pull request. A screenshot of an example setting:

@zmughal done!

This is per discussion at: - <#374 (comment)>, - <#375>.

This tests against specific Debian releases using all Perl module dependencies that can be installed via `apt-get`. This is per discussion at <bioperl#374 (comment)>.

This tests against specific Debian releases using all Perl module dependencies that can be installed via `apt-get`. This is per discussion at <#374 (comment)>.

Travis changed its terms and it's no longer accessible. We have been using Github actions (since dfbf04c and #374) and see also #325.

zmughal force-pushed the ci-github branch 3 times, most recently from c61ca82 to 893eb70 Compare March 30, 2023 23:17

zmughal marked this pull request as draft March 30, 2023 23:22

zmughal force-pushed the ci-github branch 2 times, most recently from 92cf0f0 to 1c00d90 Compare March 31, 2023 00:03

zmughal marked this pull request as ready for review March 31, 2023 00:03

Add initial GitHub Actions workflow

08f1d26

zmughal force-pushed the ci-github branch from 1c00d90 to 541d8b6 Compare March 31, 2023 00:23

Enable author testing

e0d52a9

zmughal force-pushed the ci-github branch from 1ebfca0 to 8981af0 Compare March 31, 2023 02:57

Enable testing with coverage

3a3208f

This uses Coveralls for coverage reporting.

zmughal force-pushed the ci-github branch from 8981af0 to 3a3208f Compare March 31, 2023 02:58

hlapp reviewed Apr 15, 2023

View reviewed changes

zmughal mentioned this pull request Apr 19, 2023

Failing t/SeqIO/bsml_sax.t due to 404 for DTD URL #376

Closed

cjfields merged commit 80183ef into bioperl:master Apr 26, 2023

zmughal deleted the ci-github branch April 27, 2023 02:28

zmughal added a commit to zmughal-contrib/bioperl-live that referenced this pull request Apr 27, 2023

CI: test with minimum, median, and latest Perl versions

9ebb945

This is per discussion at: - <bioperl#374 (comment)>, - <bioperl#375>.

zmughal mentioned this pull request Apr 27, 2023

CI: test with minimum, median, and latest Perl versions #378

Merged

zmughal mentioned this pull request Apr 27, 2023

CI: Add test jobs using Debian containers #379

Merged

cjfields pushed a commit that referenced this pull request May 10, 2023

CI: test with minimum, median, and latest Perl versions

7adc3aa

This is per discussion at: - <#374 (comment)>, - <#375>.

cjfields pushed a commit that referenced this pull request May 10, 2023

CI: Add test jobs using Debian containers

38073ee

This tests against specific Debian releases using all Perl module dependencies that can be installed via `apt-get`. This is per discussion at <#374 (comment)>.

carandraug added a commit that referenced this pull request Apr 26, 2024

README.md: change test suite badge from travis to Github actions.

ffd0c21

Travis changed its terms and it's no longer accessible. We have been using Github actions (since dfbf04c and #374) and see also #325.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial GitHub Actions workflow #374

Add initial GitHub Actions workflow #374

zmughal commented Mar 30, 2023 •

edited

Loading

zmughal commented Mar 30, 2023

zmughal commented Mar 31, 2023

zmughal commented Apr 1, 2023

cjfields commented Apr 14, 2023 •

edited

Loading

hlapp Apr 15, 2023

zmughal Apr 15, 2023

zmughal Apr 15, 2023

cjfields Apr 18, 2023

hlapp commented Apr 15, 2023

zmughal commented Apr 18, 2023

carandraug commented Apr 18, 2023

cjfields commented Apr 18, 2023

cjfields commented Apr 18, 2023

zmughal commented Apr 18, 2023

cjfields commented Apr 18, 2023

zmughal commented Apr 19, 2023

zmughal commented Apr 24, 2023

cjfields commented Apr 26, 2023

zmughal commented Apr 28, 2023

cjfields commented Apr 28, 2023

Add initial GitHub Actions workflow #374

Add initial GitHub Actions workflow #374

Conversation

zmughal commented Mar 30, 2023 • edited Loading

zmughal commented Mar 30, 2023

zmughal commented Mar 31, 2023

zmughal commented Apr 1, 2023

cjfields commented Apr 14, 2023 • edited Loading

hlapp Apr 15, 2023

Choose a reason for hiding this comment

zmughal Apr 15, 2023

Choose a reason for hiding this comment

zmughal Apr 15, 2023

Choose a reason for hiding this comment

cjfields Apr 18, 2023

Choose a reason for hiding this comment

hlapp commented Apr 15, 2023

zmughal commented Apr 18, 2023

carandraug commented Apr 18, 2023

cjfields commented Apr 18, 2023

cjfields commented Apr 18, 2023

zmughal commented Apr 18, 2023

cjfields commented Apr 18, 2023

zmughal commented Apr 19, 2023

zmughal commented Apr 24, 2023

cjfields commented Apr 26, 2023

zmughal commented Apr 28, 2023

cjfields commented Apr 28, 2023

zmughal commented Mar 30, 2023 •

edited

Loading

cjfields commented Apr 14, 2023 •

edited

Loading