From b2d04247ccb5bc1961b2c51a5e6ebdf992c79b2a Mon Sep 17 00:00:00 2001
From: Keith James <kdj@sanger.ac.uk>
Date: Fri, 25 Aug 2023 09:11:59 +0100
Subject: [PATCH 01/10] Change CI workflow to standard file name

---
 .../workflows/{testing_and_building_repos.yml => run-tests.yml}   | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename .github/workflows/{testing_and_building_repos.yml => run-tests.yml} (100%)

diff --git a/.github/workflows/testing_and_building_repos.yml b/.github/workflows/run-tests.yml
similarity index 100%
rename from .github/workflows/testing_and_building_repos.yml
rename to .github/workflows/run-tests.yml

From dbaf4896e10d551d8f9bb6b3431e8f0644c053cb Mon Sep 17 00:00:00 2001
From: Keith James <kdj@sanger.ac.uk>
Date: Fri, 25 Aug 2023 09:33:30 +0100
Subject: [PATCH 02/10] Change to Perl versions from Perlbrew

---
 .github/workflows/perlbrew.sha256   |   1 +
 .github/workflows/run-tests.yml     | 152 +++++++++++++++++-----------
 MANIFEST                            |   2 -
 scripts/before_install.sh           |  44 --------
 scripts/install_wsi_dependencies.sh |  54 ++++++++++
 5 files changed, 147 insertions(+), 106 deletions(-)
 create mode 100644 .github/workflows/perlbrew.sha256
 delete mode 100755 scripts/before_install.sh
 create mode 100755 scripts/install_wsi_dependencies.sh

diff --git a/.github/workflows/perlbrew.sha256 b/.github/workflows/perlbrew.sha256
new file mode 100644
index 00000000..d9992912
--- /dev/null
+++ b/.github/workflows/perlbrew.sha256
@@ -0,0 +1 @@
+c3996e4fae37a0ae01839cdd73752fb7b17e81bac2a8b39712463a7d518c4945  perlbrew.sh
diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml
index db77c54d..348ddad4 100644
--- a/.github/workflows/run-tests.yml
+++ b/.github/workflows/run-tests.yml
@@ -1,12 +1,26 @@
-name: testing_and_building_repo
+name: "Unit tests"
+
 on: [push, pull_request]
+
 jobs:
-  build: 
+  build:
+    runs-on: ubuntu-latest
+
+    defaults:
+      run:
+        shell: bash -l -e -o pipefail {0}
+
+    env:
+      PERL_CACHE: ~/perl5 # Perlbrew and CPAN modules installed here, cached
+      NPG_LIB: ~/perl5npg # NPG modules installed here, not cached
+      WTSI_NPG_GITHUB_URL: https://github.com/wtsi-npg
+      WTSI_NPG_BUILD_BRANCH: ${GITHUB_HEAD_REF}
+
+
     strategy:
       matrix:
-        perl: ['5.26'] 
-        
-    runs-on: ubuntu-latest
+        perl: ["5.26.3"]
+
     services:
        mysql:
          image: mysql:8.0
@@ -17,79 +31,97 @@ jobs:
            MYSQL_ROOT_PASSWORD: null
            MYSQL_DATABASE: npgt
          options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=10s --health-retries=5 
- 
-    name: Perl ${{ matrix.perl }}
+
     steps:
-    - uses: actions/checkout@v3
+      - uses: actions/checkout@v3
     
-    - name: Change dbhost to 127.0.0.1
-      run: |
-          sed -i s/localhost/127.0.0.1/ ${GITHUB_WORKSPACE}/data/config.ini
-
-    - name: set timezone
-      run: |
-          sudo timedatectl set-timezone Europe/London 
-  
-    # Caching cpanm external modules
-    - name: Cache cpanm external modules
-      id: cpanmCache
-      uses: actions/cache@v3
-      with:
-        path: ~/perl5ext
-        key: ${{ runner.os }}-build-cpanm-external
-
-    #install libgd-dev and uuid-dev
-    - name: install libgd-dev and uuid-dev 
-      run: |
+      - name: Change dbhost to 127.0.0.1
+        run: |
+            sed -i s/localhost/127.0.0.1/ ${GITHUB_WORKSPACE}/data/config.ini
+
+      - name: set timezone
+        run: |
+            sudo timedatectl set-timezone Europe/London 
+
+      - name: "Install OS dependencies"
+        run: |
           sudo apt-get update
           # https://github.com/actions/runner-images/issues/2139
           sudo apt-get remove -y nginx libgd3
           sudo apt-get install -y libgd-dev uuid-dev libgd-text-perl
-    
-    - name: install cpanm
-      run: |
-          wget -qO - https://cpanmin.us | /usr/bin/perl - --sudo App::cpanminus
-
-    - name: running install scripts 
-      run: |
-          cpanm --local-lib=~/perl5ext local::lib && eval $(perl -I ~/perl5ext/lib/perl5/ -Mlocal::lib)
-          ${GITHUB_WORKSPACE}/scripts/before_install.sh $WTSI_NPG_GITHUB_URL $WTSI_NPG_BUILD_BRANCH
+
+      - name: "Cache Perl"
+        id: cache-perl
+        uses: actions/cache@v3
+        with:
+          path: ${{ env.PERL_CACHE }}
+          key: ${{ runner.os }}-${{ matrix.perl }}-perl
+
+      - name: "Install Perlbrew"
+        if: steps.cache-perl.outputs.cache-hit != 'true'
+        run: |
+          curl -sSL https://install.perlbrew.pl -o perlbrew.sh
+          sha256sum -c .github/workflows/perlbrew.sha256
+          export PERLBREW_ROOT=${{ env.PERL_CACHE }}
+          sh perlbrew.sh
+            
+          source ${{ env.PERL_CACHE }}/etc/bashrc
+          perlbrew available
+          perlbrew install --notest perl-${{ matrix.perl }}
+          perlbrew use perl-${{ matrix.perl }}
+          perlbrew install-cpanm
+
+      - name: "Initialize Perlbrew"
+        run: |
+          echo "source ${{ env.PERL_CACHE }}/etc/bashrc" >> "$HOME/.bash_profile"
+
+      - name: "Install Perl dependencies"
+        run: |
+          cpanm --local-lib=${{ env.PERL_CACHE }} local::lib
+          eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib="$NPG_LIB")
+          eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib)
+          
+          cpanm --quiet --notest Module::Build
+          cpanm --quiet --notest Alien::Tidyp
           
-      env:
-        WTSI_NPG_GITHUB_URL: https://github.com/wtsi-npg
-        WTSI_NPG_BUILD_BRANCH: ${GITHUB_HEAD_REF}
-
-    - name: install cpanm dependencies
-      run: |
-          eval $(perl -I ~/perl5ext/lib/perl5/ -Mlocal::lib=~/perl5npg)
-          eval $(perl -I ~/perl5ext/lib/perl5/ -Mlocal::lib=~/perl5ext)
+          ./scripts/install_wsi_dependencies.sh "$NPG_LIB" \
+             perl-dnap-utilities \
+             ml_warehouse
+
           cpanm --installdeps --notest .
 
-    - name: run Build.PL and ./Build
-      run: |
-          eval $(perl -I ~/perl5ext/lib/perl5/ -Mlocal::lib=~/perl5ext)
-          eval $(perl -I ~/perl5ext/lib/perl5/ -Mlocal::lib=~/perl5npg)
+      - name: "Log install failure"
+        if: ${{ failure() }}
+        run: |
+          find ~/.cpanm/work -cmin -1 -name '*.log' -exec tail -n20  {} \;
+
+      - name: "Archive CPAN logs on failure"
+        if: ${{ failure() }}
+        uses: actions/upload-artifact@v2
+        with:
+          name: cpan_log
+          path: ~/.cpanm/work/*/build.log
+          retention-days: 5
+
+      - name: "Run tests"
+        run: |
+          eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib)
+          eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib="$NPG_LIB")
+          
           export TEST_AUTHOR=1
           perl Build.PL
           ./Build test --verbose
           ./Build install
 
-    - name: run ./Build 
-      run: |
-          eval $(perl -I ~/perl5ext/lib/perl5/ -Mlocal::lib=~/perl5ext)
+      - name: "Build distribution"
+        run: |
+          eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib)
+          eval $(perl -I ${{ env.PERL_CACHE }}/lib/perl5/ -Mlocal::lib="$NPG_LIB")
           export TEST_AUTHOR=1
+          
           ./Build dist
           export DIST_FILE=$(ls npg-tracking-*.tar.gz)
           export MD5_FILE=$DIST_FILE.md5
           md5sum $DIST_FILE > $MD5_FILE
           export SHA256_FILE=$DIST_FILE.sha256
           shasum -a 256 $DIST_FILE > $SHA256_FILE
-
-    # Archive logs if failure
-    - name: Archive CPAN logs
-      if: ${{ failure() }}
-      uses: actions/upload-artifact@v2
-      with:
-        name: cpan_log
-        path: /home/runner/.cpanm/work/*/build.log
-        retention-days: 5
diff --git a/MANIFEST b/MANIFEST
index e67a01cf..695e310e 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -1,4 +1,3 @@
-.github/workflows/testing_and_building_repos.yml
 bin/event_notifications
 bin/illumina_instruments_uptime
 bin/npg_daemon_control
@@ -1468,6 +1467,5 @@ t/useragent.pm
 t/util.pm
 wtsi_local/httpd.conf
 wtsi_local/httpd_sfweb.conf
-
 META.yml
 META.json
diff --git a/scripts/before_install.sh b/scripts/before_install.sh
deleted file mode 100755
index 66dc6bc6..00000000
--- a/scripts/before_install.sh
+++ /dev/null
@@ -1,44 +0,0 @@
-#!/bin/bash
-
-WTSI_NPG_GITHUB_URL=$1
-WTSI_NPG_BUILD_BRANCH=$2
-
-eval $(perl -I ~/perl5ext/lib/perl5/ -Mlocal::lib=~/perl5ext)
-cpanm --quiet --notest Module::Build
-cpanm --quiet --notest Alien::Tidyp
-
-# WTSI NPG Perl repo dependencies
-repos=""
-for repo in perl-dnap-utilities ml_warehouse; do
-    cd /tmp
-    # Always clone master when using depth 1 to get current tag
-    git clone --branch master --depth 1 ${WTSI_NPG_GITHUB_URL}/${repo}.git ${repo}.git
-    cd /tmp/${repo}.git
-    # Shift off master to appropriate branch (if possible)
-    git ls-remote --heads --exit-code origin ${WTSI_NPG_BUILD_BRANCH} && git pull origin ${WTSI_NPG_BUILD_BRANCH} && echo "Switched to branch ${WTSI_NPG_BUILD_BRANCH}"
-    repos=$repos" /tmp/${repo}.git"
-done
-
-for repo in $repos
-do
-    export PERL5LIB=$repo/blib/lib:$PERL5LIB:$repo/lib
-done
-
-for repo in $repos
-do
-    cd $repo
-    cpanm  --quiet --notest --installdeps .
-    perl Build.PL
-    ./Build
-done
-
-# Finally, bring any common dependencies up to the latest version and
-# install
-eval $(perl -I ~/perl5ext/lib/perl5/ -Mlocal::lib=~/perl5npg)
-for repo in $repos
-do
-    cd $repo
-    cpanm  --quiet --notest --installdeps .
-    ./Build install
-done
-cd
diff --git a/scripts/install_wsi_dependencies.sh b/scripts/install_wsi_dependencies.sh
new file mode 100755
index 00000000..9d1e506f
--- /dev/null
+++ b/scripts/install_wsi_dependencies.sh
@@ -0,0 +1,54 @@
+#!/bin/bash
+
+set -e -u -x
+
+WSI_NPG_GITHUB_URL=${WSI_NPG_GITHUB_URL:=https://github.com/wtsi-npg}
+WSI_NPG_BUILD_BRANCH=${WSI_NPG_BUILD_BRANCH:=devel}
+
+# The first argument is the install base for NPG modules, enabling them to be
+# installed independently of CPAN dependencies. E.g. for cases where we want
+# different caching behaviour.
+NPG_ROOT="$1"
+shift
+
+repos=""
+for repo in "$@" ; do
+    cd /tmp
+
+    # Clone deeper than depth 1 to get the tag even if something has been already
+    # committed over the tag
+    git clone --branch master --depth 3 "$WSI_NPG_GITHUB_URL/${repo}.git" "${repo}.git"
+    cd "/tmp/${repo}.git"
+
+    # Shift off master to appropriate branch (if possible)
+    git ls-remote --heads --exit-code origin "$WSI_NPG_BUILD_BRANCH" && \
+	    git pull origin "$WSI_NPG_BUILD_BRANCH" && \
+	    echo "Switched to branch $WSI_NPG_BUILD_BRANCH"
+    repos="$repos /tmp/${repo}.git"
+done
+
+# Install CPAN dependencies. The src libs are on PERL5LIB because of
+# circular dependencies. The blibs are on PERL5LIB because the package
+# version, which cpanm requires, is inserted at build time. They must
+# be before the libs for cpanm to pick them up in preference.
+for repo in $repos
+do
+    export PERL5LIB="$PERL5LIB:$repo/blib/lib:$repo/lib"
+done
+
+for repo in $repos
+do
+    cd "$repo"
+    cpanm --quiet --notest --installdeps .
+    perl Build.PL
+    ./Build
+done
+
+# Finally, bring any common dependencies up to the latest version and
+# install
+for repo in $repos
+do
+    cd "$repo"
+    cpanm --quiet --notest --installdeps .
+    ./Build install --install-base "$NPG_ROOT"
+done

From cfbdd542d5ed646d5844de4e8a2e977e7e184f81 Mon Sep 17 00:00:00 2001
From: Keith James <kdj@sanger.ac.uk>
Date: Fri, 25 Aug 2023 11:11:34 +0100
Subject: [PATCH 03/10] Add Perl 5.34.1 to the test matrix

---
 .github/workflows/run-tests.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml
index 348ddad4..8f4b9ab3 100644
--- a/.github/workflows/run-tests.yml
+++ b/.github/workflows/run-tests.yml
@@ -19,7 +19,7 @@ jobs:
 
     strategy:
       matrix:
-        perl: ["5.26.3"]
+        perl: ["5.26.3", "5.34.1"]
 
     services:
        mysql:

From 3ff125954036261aca84e6924158fff3e5f034b8 Mon Sep 17 00:00:00 2001
From: Marina Gourtovaia <mg8@sanger.ac.uk>
Date: Tue, 29 Aug 2023 18:05:49 +0100
Subject: [PATCH 04/10] Switched default LIMS driver in samplesheet generation

Switched the default LIMS driver in samplesheet generation
from 'xml' to 'ml_warehouse'.
Added 'lims_driver_type' attribute to npg::samplesheet.
Updated the tests for samplesheet generation, which are using
the xml LIMS driver, to set the driver type explicitly.
---
 Changes                |  6 ++++++
 lib/npg/samplesheet.pm | 18 +++++++++++++++---
 t/47-samplesheet.t     | 32 ++++++++++++++++----------------
 3 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/Changes b/Changes
index 319702e2..65a94094 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,11 @@
 LIST OF CHANGES
 
+ - Switched the default LIMS driver in samplesheet generation from 'xml'
+   to 'ml_warehouse'.
+ - Added 'lims_driver_type' attribute to npg::samplesheet.
+ - Updated the tests for samplesheet generation, which are using the xml
+   LIMS driver, to set the driver type explicitly.
+
 release 96.0.0
  - Fixed a regression in the npg_move_runfolder script,
    which made it unusable.
diff --git a/lib/npg/samplesheet.pm b/lib/npg/samplesheet.pm
index cbed3db2..0008d5f7 100755
--- a/lib/npg/samplesheet.pm
+++ b/lib/npg/samplesheet.pm
@@ -62,12 +62,25 @@ still retained in the relevant custom fields.
 my$config=get_config_staging_areas();
 Readonly::Scalar my $SAMPLESHEET_PATH => $config->{'samplesheets'}||q(samplesheets/);
 Readonly::Scalar my $MIN_COLUMN_NUM   => 3;
-Readonly::Scalar my $DEFAULT_LIMS_DRIVER_TYPE => 'xml';
+Readonly::Scalar my $DEFAULT_LIMS_DRIVER_TYPE => 'ml_warehouse';
 
 ##################################################################
 ####################### Public attributes ########################
 ##################################################################
 
+=head2 lims_driver_type
+
+LIMs driver type to use, defaults to ml_warehouse.
+
+=cut
+
+has 'lims_driver_type' => (
+  'isa'      => 'Str',
+  'required' => 0,
+  'is'       => 'ro',
+  'default'  => $DEFAULT_LIMS_DRIVER_TYPE,
+);
+
 =head2 id_run
 
 An optional attribute
@@ -180,7 +193,6 @@ An attribute, an array of st::api::lims type objects.
 
 This attribute should normally be provided by the caller via the
 constuctor. If the attribute is not provided, it it built automatically.
-XML st::api::lims driver is used to access LIMS data.
 
 =cut
 
@@ -201,7 +213,7 @@ sub _build_lims {
   }
   return [st::api::lims->new(
             batch_id => $id,
-            driver_type => $DEFAULT_LIMS_DRIVER_TYPE)->children];
+            driver_type => $self->lims_driver_type)->children];
 };
 
 =head2 output
diff --git a/t/47-samplesheet.t b/t/47-samplesheet.t
index c8a5964e..9e676ab7 100644
--- a/t/47-samplesheet.t
+++ b/t/47-samplesheet.t
@@ -23,18 +23,18 @@ subtest 'object creation' => sub {
   plan tests => 8;
 
   my $result = q();
-  dies_ok { npg::samplesheet->new( repository=>$dir, output=>\$result)->process }
+  dies_ok { npg::samplesheet->new( lims_driver_type=>'xml', repository=>$dir, output=>\$result)->process }
     'sample sheet process fails when neither run object nor id_run given';
 
   my $ss;
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007); } 'sample sheet object - no output provided';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007); } 'sample sheet object - no output provided';
   cmp_ok($ss->output, 'eq', '/nfs/sf49/ILorHSorMS_sf49/samplesheets/wibble/MS0001309-300.csv', 'default output location (with zeroes trimmed appropriately)');
   is($ss->lims->[0]->driver_type, 'xml', 'xml driver is used');
 
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946); } 'sample sheet object - no output provided';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946); } 'sample sheet object - no output provided';
   cmp_ok($ss->output, 'eq', '/nfs/sf49/ILorHSorMS_sf49/samplesheets/wibble/000000000-A0616.csv', 'default output location');
 
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007); } 'sample sheet object - no output provided';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007); } 'sample sheet object - no output provided';
   my $orig_flowcell_id = $ss->run->flowcell_id;
   $ss->run->flowcell_id(q(MS2000132-500V2));
   cmp_ok($ss->output, 'eq', '/nfs/sf49/ILorHSorMS_sf49/samplesheets/wibble/MS2000132-500V2.csv', 'default output location copes with V2 MiSeq cartirdges/reagent kits');
@@ -91,13 +91,13 @@ RESULT_7007
 
   my $ss;
   my $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007, output=>\$result); } 'sample sheet object for unplexed paired run';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7007, output=>\$result); } 'sample sheet object for unplexed paired run';
   lives_ok { $ss->process(); } ' sample sheet generated';
   is_string($result, $expected_result_7007);
 
   my $run = $schema->resultset(q(Run))->find(7007);
   $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, run=>$run, output=>\$result); } 'sample sheet object from run object - no id_run given';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, run=>$run, output=>\$result); } 'sample sheet object from run object - no id_run given';
   lives_ok { $ss->process(); } ' sample sheet generated';
   is_string($result, $expected_result_7007);
 };
@@ -107,7 +107,7 @@ subtest 'default samplesheet for a plexed paired run' => sub {
 
   my $ss;
   my $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, output=>\$result); } 'samplesheet object for plexed paired run';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, output=>\$result); } 'samplesheet object for plexed paired run';
   my $expected_result = << 'RESULT_6946';
 [Header],,,,
 Investigator Name,mq1,,,
@@ -150,7 +150,7 @@ subtest 'default samplesheet for a plexed paired run with reference fallback' =>
 
   my $ss;
   my $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7825, output=>\$result); } 'sample sheet object for plexed paired run';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7825, output=>\$result); } 'sample sheet object for plexed paired run';
   my $expected_result = << 'RESULT_7825';
 [Header],,,,
 Investigator Name,nh4,,,
@@ -187,7 +187,7 @@ subtest 'default samplesheet, mkfastq option enabled' => sub {
   # with the mkfastq option we get an extra leading column, Lane
   my $ss;
   my $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7826, mkfastq => 1, output=>\$result); }
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7826, mkfastq => 1, output=>\$result); }
     'sample sheet object mkfastq';
   my $expected_result = << 'RESULT_mkfastq';
 [Header],,,,,
@@ -224,7 +224,7 @@ subtest 'default samplesheet for dual index' => sub {
 
   my $ss;
   my $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>7826, output=>\$result); } 'sample sheet object for dual index';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>7826, output=>\$result); } 'sample sheet object for dual index';
   my $expected_result = << 'RESULT_7826';
 [Header],,,,
 Investigator Name,nh4,,,
@@ -260,13 +260,13 @@ subtest 'extended samplesheets' => sub {
 
   my $ss;
   my $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, extend => 1, id_run=>7007, output=>\$result); } 'extended sample sheet object for unplexed paired run';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, extend => 1, id_run=>7007, output=>\$result); } 'extended sample sheet object for unplexed paired run';
   ok(!$ss->_dual_index, 'no dual index');
   lives_ok { $ss->process(); } ' sample sheet generated';
   is_string($result, read_file('t/data/samplesheet/7007_extended.csv'));
 
   $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } 'extended sample sheet object for plexed paired run';
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); } 'extended sample sheet object for plexed paired run';
   ok(!$ss->_dual_index, 'no dual index');
   lives_ok { $ss->process(); } ' sample sheet generated';
   is_string($result, read_file('t/data/samplesheet/6946_extended.csv'));
@@ -276,7 +276,7 @@ subtest 'extended samplesheets' => sub {
   $schema->resultset('Run')->find(6946)->update({batch_id => 4775});
 
   $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); }
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); }
     'extended sample sheet object for unplexed paired 8 lane run with a control lane';
   lives_ok { $ss->process(); } 'sample sheet generated';
   is_string($result, read_file('t/data/samplesheet/1control7libs_extended.csv'));
@@ -286,7 +286,7 @@ subtest 'extended samplesheets' => sub {
   $schema->resultset('Run')->find(6946)->update({batch_id => 16249});
 
   $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); }
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); }
     'extended sample sheet object for plexed paired 8 lane run';
   ok(!$ss->_dual_index, 'no dual index');
   lives_ok { $ss->process(); } 'sample sheet generated';
@@ -297,7 +297,7 @@ subtest 'extended samplesheets' => sub {
   $schema->resultset('Run')->find(6946)->update({batch_id => 23798});
 
   $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); }
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); }
     'extended sample sheet object for plexed paired run with both pool and library lanes';
   ok($ss->_dual_index, 'dual index from a 16 char first index');
   lives_ok { $ss->process(); } 'sample sheet generated';
@@ -307,7 +307,7 @@ subtest 'extended samplesheets' => sub {
   $schema->resultset('Run')->find(6946)->update({batch_id => 1,});
 
   $result = q();
-  lives_ok { $ss = npg::samplesheet->new(repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); }
+  lives_ok { $ss = npg::samplesheet->new(lims_driver_type=>'xml', repository=>$dir, npg_tracking_schema=>$schema, id_run=>6946, extend => 1, output=>\$result); }
     'extended sample sheet object for plexed paired run with both pool and library lanes';
   ok($ss->_dual_index, 'dual index from two indexes in LIMs');
   lives_ok { $ss->process(); } 'sample sheet generated';

From 19e1f371598ca68dc52fd794f18130467e17ccc6 Mon Sep 17 00:00:00 2001
From: Marina Gourtovaia <mg8@sanger.ac.uk>
Date: Wed, 30 Aug 2023 11:44:12 +0100
Subject: [PATCH 05/10] Dropped a special case of batch-less pools

---
 Changes                | 2 ++
 lib/npg/samplesheet.pm | 7 -------
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/Changes b/Changes
index 65a94094..c5a1422d 100644
--- a/Changes
+++ b/Changes
@@ -5,6 +5,8 @@ LIST OF CHANGES
  - Added 'lims_driver_type' attribute to npg::samplesheet.
  - Updated the tests for samplesheet generation, which are using the xml
    LIMS driver, to set the driver type explicitly.
+ - In samplesheet generation, dropped a special case of MiSeq instruments
+   without a batch; last use in production was over 5 years ago.
 
 release 96.0.0
  - Fixed a regression in the npg_move_runfolder script,
diff --git a/lib/npg/samplesheet.pm b/lib/npg/samplesheet.pm
index 0008d5f7..5d7f3873 100755
--- a/lib/npg/samplesheet.pm
+++ b/lib/npg/samplesheet.pm
@@ -204,13 +204,6 @@ has 'lims' => (
 sub _build_lims {
   my $self=shift;
   my $id = $self->run->batch_id;
-  if ($id=~/\A\d{13}\z/smx) {
-    load_class 'st::api::lims::warehouse';
-    return [st::api::lims->new(
-      position => 1,
-      driver   => st::api::lims::warehouse->new(position=>1, tube_ean13_barcode=>$id)
-    )];
-  }
   return [st::api::lims->new(
             batch_id => $id,
             driver_type => $self->lims_driver_type)->children];

From 626bd65f94ed2258f9ade06960e374d88355bff2 Mon Sep 17 00:00:00 2001
From: Marina Gourtovaia <mg8@sanger.ac.uk>
Date: Wed, 30 Aug 2023 12:40:57 +0100
Subject: [PATCH 06/10] Fixed method for building lims objects

---
 lib/npg/samplesheet.pm | 38 +++++++++++++++++++++++++++++++++-----
 t/47-samplesheet.t     | 42 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/lib/npg/samplesheet.pm b/lib/npg/samplesheet.pm
index 5d7f3873..85e69743 100755
--- a/lib/npg/samplesheet.pm
+++ b/lib/npg/samplesheet.pm
@@ -15,6 +15,7 @@ use st::api::lims;
 use st::api::lims::samplesheet;
 use npg_tracking::util::config qw(get_config_staging_areas);
 use npg_tracking::util::abs_path qw(abs_path);
+use WTSI::DNAP::Warehouse::Schema;
 
 with 'npg_tracking::glossary::run';
 
@@ -170,6 +171,21 @@ sub _build_npg_tracking_schema {
   return $s
 }
 
+=head2 mlwh_schema
+ 
+DBIx schema class for ml_warehouse access.
+
+=cut
+
+has 'mlwh_schema' => (
+                isa        => 'WTSI::DNAP::Warehouse::Schema',
+                is         => 'ro',
+                required   => 0,
+                lazy_build => 1,);
+sub _build_mlwh_schema {
+  return WTSI::DNAP::Warehouse::Schema->connect();
+}
+
 =head2 run
 
 An attribute, DBIx object for a row in the run table of the tracking database.
@@ -203,10 +219,20 @@ has 'lims' => (
 );
 sub _build_lims {
   my $self=shift;
-  my $id = $self->run->batch_id;
-  return [st::api::lims->new(
-            batch_id => $id,
-            driver_type => $self->lims_driver_type)->children];
+
+  my $ref = {driver_type => $self->lims_driver_type};
+  my $batch_id = $self->run->batch_id;
+  if ($self->lims_driver_type eq $DEFAULT_LIMS_DRIVER_TYPE) {
+    $ref->{'id_flowcell_lims'} = $batch_id;
+    $ref->{'mlwh_schema'} = $self->mlwh_schema;
+  } elsif ($self->lims_driver_type eq 'xml') {
+    $ref->{'batch_id'} = $batch_id;
+  } else {
+    croak sprintf 'Lazy-build for driver type %s is not inplemented',
+      $self->lims_driver_type;
+  }
+
+  return [st::api::lims->new($ref)->children];
 };
 
 =head2 output
@@ -597,6 +623,8 @@ __END__
 
 =item open
 
+=item WTSI::DNAP::Warehouse::Schema
+
 =back
 
 =head1 INCOMPATIBILITIES
@@ -609,7 +637,7 @@ David K. Jackson E<lt>david.jackson@sanger.ac.ukE<gt>
 
 =head1 LICENSE AND COPYRIGHT
 
-Copyright (C) 2019,2020 Genome Research Ltd.
+Copyright (C) 2019,2020, 2023 Genome Research Ltd.
 
 This file is part of NPG.
 
diff --git a/t/47-samplesheet.t b/t/47-samplesheet.t
index 9e676ab7..4b88dea3 100644
--- a/t/47-samplesheet.t
+++ b/t/47-samplesheet.t
@@ -1,11 +1,12 @@
 use strict;
 use warnings;
-use Test::More tests => 11;
+use Test::More tests => 13;
 use Test::LongString;
 use Test::Exception;
 use File::Slurp;
 use File::Temp qw/tempdir/;
 use File::Path qw/make_path/;
+use Moose::Meta::Class;
 
 use t::dbic_util;
 local $ENV{'dev'} = q(wibble); # ensure we're not going live anywhere
@@ -15,6 +16,12 @@ use_ok('npg::samplesheet');
 use_ok('st::api::lims');
 
 my $schema = t::dbic_util->new->test_schema();
+
+my $class = Moose::Meta::Class->create_anon_class(roles=>[qw/npg_testing::db/]);
+my $mlwh_schema = $class->new_object({})->create_test_db(
+  q[WTSI::DNAP::Warehouse::Schema], q[t/data/fixtures_lims_wh]
+);
+
 local $ENV{NPG_WEBSERVICE_CACHE_DIR} = q(t/data/samplesheet);
 
 my $dir = tempdir( CLEANUP => 1 );
@@ -40,6 +47,39 @@ subtest 'object creation' => sub {
   cmp_ok($ss->output, 'eq', '/nfs/sf49/ILorHSorMS_sf49/samplesheets/wibble/MS2000132-500V2.csv', 'default output location copes with V2 MiSeq cartirdges/reagent kits');
 };
 
+subtest 'error on an unknown driver types' => sub {
+  plan tests => 1;
+
+  throws_ok {
+    npg::samplesheet->new(
+      lims_driver_type    => 'foo',
+      repository          => $dir,
+      npg_tracking_schema => $schema,
+      id_run              => 7007)->lims()
+  } qr/Lazy-build for driver type foo is not inplemented/,
+  'error with the driver type for which LIMS objects cannot be built';
+};
+
+subtest 'simple tests for the default driver' => sub {
+   plan tests => 2;
+
+   my $run_row = $schema->resultset('Run')->find(7007);
+   my $current_batch_id = $run_row->batch_id;
+   $run_row->update({batch_id => 57543});
+
+   my $ss = npg::samplesheet->new(
+      repository          => $dir,
+      npg_tracking_schema => $schema,
+      mlwh_schema         => $mlwh_schema,
+      id_run              => 7007
+   );
+   is ($ss->lims_driver_type, 'ml_warehouse', 'correct default driver type');
+   my $lims = $ss->lims();
+   is (@{$lims}, 1, 'LIMS data for 1 lane is built');
+
+   $run_row->update({batch_id => $current_batch_id});
+};
+
 subtest 'values conversion' => sub {
   plan tests => 12;
 

From cc51b1d2038ada260283ff7e89c6c79b05407951 Mon Sep 17 00:00:00 2001
From: Marina Gourtovaia <mg8@sanger.ac.uk>
Date: Wed, 30 Aug 2023 13:13:07 +0100
Subject: [PATCH 07/10] Stopped access to a live db from a test

---
 t/60-illumina-runfolder.t | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/t/60-illumina-runfolder.t b/t/60-illumina-runfolder.t
index b0258ac7..41d5a51d 100644
--- a/t/60-illumina-runfolder.t
+++ b/t/60-illumina-runfolder.t
@@ -13,12 +13,12 @@ use t::dbic_util;
 
 use_ok('npg_tracking::illumina::runfolder');
 
-my $CLEANUP=1;
+my $schema = t::dbic_util->new->test_schema();
 
 my $testrundata = catfile(rel2abs(dirname(__FILE__)),q(data),q(090414_IL24_2726.tar.bz2));
 
 my $origdir = getcwd;
-my $testdir = catfile(tempdir( CLEANUP => $CLEANUP ), q()); #we need a trailing slash after our directory for the globbing later
+my $testdir = catfile(tempdir( CLEANUP => 1 ), q()); #we need a trailing slash after our directory for the globbing later
 chdir $testdir or die "Cannot change to temporary directory $testdir";
 diag ("Extracting $testrundata to $testdir");
 system('tar','xjf',$testrundata) == 0 or Archive::Tar->extract_archive($testrundata, 1);
@@ -125,7 +125,6 @@ ENDXML
   my $new_name = q(4567XXTT98);
   my $testrundir_new = catdir($testdir, $new_name);
   rename $testrundir, $testrundir_new;
-  my $schema = t::dbic_util->new->test_schema();
   my $id_run = 33333;
   my $data = {
     actual_cycle_count   => 30,
@@ -192,8 +191,6 @@ subtest 'getting id_run from experiment name in run parameters' => sub {
   my $rf = join q[/], $basedir, 'runfolder_id_run';
   mkdir $rf;
 
-  use npg_tracking::illumina::runfolder;
-
   my %data = (
     'runParameters.hiseq4000.xml'       => { 'rpf' => 'runParameters', 'expname' => '24359' },
     'runParameters.hiseq.rr.single.xml' => { 'rpf' => 'runParameters', 'expname' => '25835' },
@@ -203,7 +200,6 @@ subtest 'getting id_run from experiment name in run parameters' => sub {
     'runParameters.hiseq.xml'           => { 'rpf' => 'runParameters', 'expname' => '24235' },
     'runParameters.hiseqx.upgraded.xml' => { 'rpf' => 'runParameters', 'expname' => '24420' },
     'runParameters.hiseqx.xml'          => { 'rpf' => 'runParameters', 'expname' => '24422' },
-#    'RunParameters.novaseq.xml'         => { 'rpf' => 'RunParameters', 'expname' => 'Coriell_24PF_auto_PoolF_NEBreagents_TruseqAdap_500pM_NV7B' },
   );
 
   my $expname_data = \%data;
@@ -218,8 +214,10 @@ subtest 'getting id_run from experiment name in run parameters' => sub {
     use File::Copy;
     copy(join(q[/],$run_param_dir,$file_name), $run_params_file_path) or die 'Failed to copy file';
 
-    use npg_tracking::illumina::runfolder;
-    my $li = new npg_tracking::illumina::runfolder( runfolder_path => $rf );
+    my $li = new npg_tracking::illumina::runfolder(
+      runfolder_path => $rf,
+      npg_tracking_schema => $schema
+    );
 
     is($li->id_run(), $expected_experiment_name, q[Expected id_run parsed from experiment name in run params]);
     `rm $run_params_file_path`

From 15806dfa1e1f76d0dd76274d8cd491e6a77b8498 Mon Sep 17 00:00:00 2001
From: Marina Gourtovaia <mg8@sanger.ac.uk>
Date: Wed, 30 Aug 2023 14:17:51 +0100
Subject: [PATCH 08/10] Drop tests using xml lims driver.

Some tests use lims objects that are created with the xml
lims driver for comparison. This was useful at a time when
new driver types were introduced. No longer of value.
---
 t/40-st-lims-samplesheet.t | 33 ++-------------------------------
 t/40-st-lims.t             | 14 +++-----------
 2 files changed, 5 insertions(+), 42 deletions(-)

diff --git a/t/40-st-lims-samplesheet.t b/t/40-st-lims-samplesheet.t
index 2319e875..c3a10f1e 100644
--- a/t/40-st-lims-samplesheet.t
+++ b/t/40-st-lims-samplesheet.t
@@ -1,6 +1,6 @@
 use strict;
 use warnings;
-use Test::More tests => 12;
+use Test::More tests => 11;
 use Test::Exception;
 use Test::Warn;
 use File::Slurp;
@@ -337,42 +337,13 @@ subtest 'Multiple NovaSeq runs - top-up merge support' => sub {
   }
 };
 
-subtest 'MiSeq run, comparison of xml and samplesheet drivers' => sub {
-  plan tests => 4;
-
-  use_ok('st::api::lims::xml');
-  use_ok('st::api::lims');
-  my $path = 't/data/samplesheet/6946_extended.csv'; #extended MiSeq samplesheet
-  my @ss_lanes = st::api::lims::samplesheet->new(id_run => 6946, path => $path)->children;
-  local $ENV{NPG_WEBSERVICE_CACHE_DIR} = q[t/data/samplesheet];
-  my @xml_lanes = st::api::lims::xml->new(batch_id => 13994)->children;
-  my @methods = grep {$_ ne 'lane_id' && $_ ne 'lane_priority'} @st::api::lims::DELEGATED_METHODS;
-  push @methods, 'is_pool';
-
-  ok($ss_lanes[0]->is_pool, 'lane is a pool');
-  is_deeply(_lane_hash($ss_lanes[0], @methods), _lane_hash($xml_lanes[0], @methods),
-    'xml and samplesheet drivers give the same result for plexes' );
-};
-
 subtest 'multiple lanes, comparison of xml and samplesheet drivers' => sub {
-  plan tests => 7;
+  plan tests => 5;
 
   my $path = 't/data/samplesheet/4pool4libs_extended.csv';
   my @ss_lanes = st::api::lims::samplesheet->new(id_run => 6946, path => $path)->children;
-  local $ENV{NPG_WEBSERVICE_CACHE_DIR} = q[t/data/samplesheet];
-  my @xml_lanes = st::api::lims::xml->new(batch_id => 23798)->children;
-  my @methods = @st::api::lims::DELEGATED_METHODS;
-  push @methods, 'is_pool';
-
   ok(!$ss_lanes[0]->is_pool, 'lane 1 is a not pool');
-  is_deeply(_lane_hash($ss_lanes[0], @methods), _lane_hash($xml_lanes[0], @methods),
-    'xml and samplesheet drivers give the same result for a library' );
-
-  @methods = grep {$_ ne 'lane_id' && $_ ne 'lane_priority' && $_ ne 'spiked_phix_tag_index'} @methods;
   ok($ss_lanes[6]->is_pool, 'lane 7 is a pool');
-  is_deeply(_lane_hash($ss_lanes[6], @methods), _lane_hash($xml_lanes[6], @methods),
-    'xml and samplesheet drivers give the same result for plexes' );
-
   my @plexes = $ss_lanes[6]->children;
   my @spiked = grep {$_->spiked_phix_tag_index == 168} $ss_lanes[6]->children;
   is (scalar @spiked, scalar @plexes, 'spiked_phix_tag_index is set on plex level');
diff --git a/t/40-st-lims.t b/t/40-st-lims.t
index eb2b8e36..8a9b0193 100644
--- a/t/40-st-lims.t
+++ b/t/40-st-lims.t
@@ -998,18 +998,10 @@ subtest 'Instantiating a samplesheet driver' => sub {
 };
 
 subtest 'Dual index' => sub {
-  plan tests => 32;
+  plan tests => 16;
 
   local $ENV{NPG_CACHED_SAMPLESHEET_FILE} = 't/data/samplesheet/dual_index_extended.csv';
-  _test_di( st::api::lims->new(id_run => 6946) );
-
-  local $ENV{NPG_WEBSERVICE_CACHE_DIR} = 't/data/samplesheet';
-  _test_di( st::api::lims->new(batch_id => 1, %dr) );
-};
-
-sub _test_di {
-  my $l = shift;
-
+  my $l = st::api::lims->new(id_run => 6946);
   my @lanes = $l->children;
   is (scalar @lanes, 2, 'two lanes');
   my @plexes = $lanes[0]->children;
@@ -1033,7 +1025,7 @@ sub _test_di {
   is($plex->default_tagtwo_sequence, 'GGGGGGGG', 'second index');
   is($plex->tag_sequence, 'GTCTTGGCGGGGGGGG', 'combined tag sequence');
   is($plex->purpose, 'standard', 'purpose');
-}
+};
 
 subtest 'aggregation across lanes for pools' => sub {
   plan tests => 85;

From 8b15569a82d9703557171106b082a243accc6a85 Mon Sep 17 00:00:00 2001
From: Marina Gourtovaia <mg8@sanger.ac.uk>
Date: Thu, 31 Aug 2023 13:50:54 +0100
Subject: [PATCH 09/10] Fixed indentation and added quoting for properties

---
 lib/npg/samplesheet.pm | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/npg/samplesheet.pm b/lib/npg/samplesheet.pm
index 85e69743..4f7d626c 100755
--- a/lib/npg/samplesheet.pm
+++ b/lib/npg/samplesheet.pm
@@ -178,10 +178,11 @@ DBIx schema class for ml_warehouse access.
 =cut
 
 has 'mlwh_schema' => (
-                isa        => 'WTSI::DNAP::Warehouse::Schema',
-                is         => 'ro',
-                required   => 0,
-                lazy_build => 1,);
+  'isa'        => 'WTSI::DNAP::Warehouse::Schema',
+  'is'         => 'ro',
+  'required'   => 0,
+  'lazy_build' => 1,
+);
 sub _build_mlwh_schema {
   return WTSI::DNAP::Warehouse::Schema->connect();
 }

From 41a7671d63adbbe286dbf0b748e4b3f16f8cefde Mon Sep 17 00:00:00 2001
From: jmtcsngr <jmtc@sanger.ac.uk>
Date: Fri, 15 Sep 2023 21:45:07 +0100
Subject: [PATCH 10/10] prep release 97.0.0

---
 Changes | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Changes b/Changes
index c5a1422d..be449358 100644
--- a/Changes
+++ b/Changes
@@ -1,5 +1,9 @@
 LIST OF CHANGES
 
+release 97.0.0
+ - Change CI workflow to standard file name
+ - Change to Perl versions from Perlbrew
+ - Add Perl 5.34.1 to the test matrix
  - Switched the default LIMS driver in samplesheet generation from 'xml'
    to 'ml_warehouse'.
  - Added 'lims_driver_type' attribute to npg::samplesheet.