Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,22 @@ jobs:
wget -qO- https://code.askimed.com/install/nf-test | bash -s ${{ env.NFT_VER }}
sudo mv nf-test /usr/local/bin/

- name: Download onsite test data directory
if: matrix.module == 'bigbio/onsite'
run: |
DIR="tests/data/onsite/Phospho_redissolve_final_01_clean_perc.idparquet"
mkdir -p "$DIR"
BASE_URL="https://ftp.pride.ebi.ac.uk/pub/databases/pride/resources/proteomes/quantms-ci-github/onsite/Phospho_redissolve_final_01_clean_perc.idparquet"
for f in psms.parquet proteins.parquet protein_groups.parquet search_params.parquet; do
echo "Downloading $f ..."
curl -sL -o "$DIR/$f" "$BASE_URL/$f"
done
Comment on lines +176 to +179

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make the onsite fixture download fail-fast and bounded.

At Line 178, curl -sL can silently succeed on HTTP errors and has no timeout/retry controls. That can produce opaque nf-test failures later or flaky CI hangs.

Suggested hardening
       - name: Download onsite test data directory
         if: matrix.module == 'bigbio/onsite'
         run: |
+          set -euo pipefail
           DIR="tests/data/onsite/Phospho_redissolve_final_01_clean_perc.idparquet"
           mkdir -p "$DIR"
           BASE_URL="https://ftp.pride.ebi.ac.uk/pub/databases/pride/resources/proteomes/quantms-ci-github/onsite/Phospho_redissolve_final_01_clean_perc.idparquet"
           for f in psms.parquet proteins.parquet protein_groups.parquet search_params.parquet; do
             echo "Downloading $f ..."
-            curl -sL -o "$DIR/$f" "$BASE_URL/$f"
+            curl --fail --show-error --location \
+              --retry 3 --retry-all-errors --retry-delay 2 \
+              --connect-timeout 15 --max-time 120 \
+              -o "$DIR/$f" "$BASE_URL/$f"
+            test -s "$DIR/$f"
           done
           ls -lh "$DIR"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 176 - 179, The download loop using
curl in the for loop (iterating over psms.parquet proteins.parquet
protein_groups.parquet search_params.parquet and using $DIR/$f and $BASE_URL/$f)
can silently succeed on HTTP errors and hang without timeouts; update the curl
invocation to fail-fast and bounded by adding -f (to treat HTTP errors as
failures), -S for error output, explicit timeouts (e.g. --connect-timeout and
--max-time), and retry limits (e.g. --retry and --retry-delay or
--retry-connrefused), and ensure the loop exits on any curl failure (e.g. by
checking curl's exit code or enabling set -euo pipefail in the script) so a bad
download fails the CI immediately.

ls -lh "$DIR"

- name: Run nf-test for ${{ matrix.module }}
env:
PROFILE: ${{ matrix.profile }}
ONSITE_IDPARQUET_PATH: ${{ github.workspace }}/tests/data/onsite/Phospho_redissolve_final_01_clean_perc.idparquet
run: |
MODULE_PATH="${{ matrix.module }}"
TEST_FILE="modules/${MODULE_PATH}/tests/main.nf.test"
Expand Down
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,9 @@ nf.test.snap
.idea/

.nf-test*

#Ignore vscode AI rules
.github\instructions\codacy.instructions.md

# Test data downloaded at CI runtime
tests/data/
2 changes: 1 addition & 1 deletion modules/bigbio/onsite/environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ channels:
- bioconda
- defaults
dependencies:
- bioconda::pyonsite=0.0.2
- bioconda::pyonsite=0.0.3
12 changes: 6 additions & 6 deletions modules/bigbio/onsite/main.nf
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ process ONSITE {
label 'onsite'

container "${workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container
? 'https://depot.galaxyproject.org/singularity/pyonsite:0.0.2--pyhdfd78af_0'
: 'biocontainers/pyonsite:0.0.2--pyhdfd78af_0'}"
? 'oras://ghcr.io/bigbio/pyonsite-sif:0.0.3'
: 'ghcr.io/bigbio/pyonsite:0.0.3'}"

input:
tuple val(meta), path(mzml_file), path(id_file)

output:
tuple val(meta), path("${prefix}_*.idXML"), emit: ptm_in_id_onsite
tuple val(meta), path("${prefix}_*.idparquet"), emit: ptm_in_id_onsite
path "versions.yml", emit: versions
path "*.log", emit: log

Expand Down Expand Up @@ -48,7 +48,7 @@ process ONSITE {
onsite ascore \\
-in ${mzml_file} \\
-id ${id_file} \\
-out ${prefix}_ascore.idXML \\
-out ${prefix}_ascore.idparquet \\
--fragment-mass-tolerance ${fragment_tolerance} \\
--fragment-mass-unit ${fragment_unit}${optional_flags ? ' \\\n ' + optional_flags : ''}
"""
Expand All @@ -61,7 +61,7 @@ process ONSITE {
onsite phosphors \\
-in ${mzml_file} \\
-id ${id_file} \\
-out ${prefix}_phosphors.idXML \\
-out ${prefix}_phosphors.idparquet \\
--fragment-mass-tolerance ${fragment_tolerance} \\
--fragment-mass-unit ${fragment_unit}${optional_flags ? ' \\\n ' + optional_flags : ''}
${args}
Expand Down Expand Up @@ -92,7 +92,7 @@ process ONSITE {
onsite lucxor \\
-in ${mzml_file} \\
-id ${id_file} \\
-out ${prefix}_lucxor.idXML \\
-out ${prefix}_lucxor.idparquet \\
--fragment-method ${fragment_method} \\
--fragment-mass-tolerance ${fragment_tolerance} \\
--fragment-error-units ${fragment_unit} \\
Expand Down
10 changes: 5 additions & 5 deletions modules/bigbio/onsite/meta.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,20 @@ input:
ontologies: []
- id_file:
type: file
description: Protein/peptide identifications file in idXML format
pattern: "*.idXML"
description: Protein/peptide identifications file in idparquet format
pattern: "*.idparquet"
ontologies: []
output:
ptm_in_id_onsite:
- - meta:
type: map
description: |
Groovy Map containing sample information
- ${prefix}_*.idXML:
- ${prefix}_*.idparquet:
type: file
description: Output idXML file containing PTM localization results from
description: Output idparquet file containing PTM localization results from
onsite
pattern: "${prefix}_*.idXML"
pattern: "${prefix}_*.idparquet"
ontologies: []
log:
- "*.log":
Expand Down
23 changes: 12 additions & 11 deletions modules/bigbio/onsite/tests/main.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ nextflow_process {
when {
process {
"""
input[0] = [
[ id: 'test', mzml_id: 'test_sample' ],
file(params.test_data['proteomics']['onsite']['mzml'], checkIfExists: true),
file(params.test_data['proteomics']['onsite']['idxml'], checkIfExists: true)
]
input[0] = channel.from([
[
[ id: 'test', mzml_id: 'test_sample' ],
file(params.test_data['proteomics']['onsite']['mzml'], checkIfExists: true)
]
]).combine(channel.fromPath(params.test_data['proteomics']['onsite']['idparquet']))
"""
}
params {
Expand All @@ -31,7 +32,7 @@ nextflow_process {
assert process.out.ptm_in_id_onsite.size() == 1
assert process.out.log.size() == 1
// Check output file has correct naming
assert process.out.ptm_in_id_onsite[0][1].toString().endsWith('_ascore.idXML')
assert process.out.ptm_in_id_onsite[0][1].toString().endsWith('_ascore.idparquet')
}
}

Expand All @@ -43,7 +44,7 @@ nextflow_process {
input[0] = [
[ id: 'test', mzml_id: 'test_sample' ],
file(params.test_data['proteomics']['onsite']['mzml'], checkIfExists: true),
file(params.test_data['proteomics']['onsite']['idxml'], checkIfExists: true)
file(params.test_data['proteomics']['onsite']['idparquet'], checkIfExists: true)
]
"""
}
Expand All @@ -58,7 +59,7 @@ nextflow_process {
assert process.out.ptm_in_id_onsite.size() == 1
assert process.out.log.size() == 1
// Check output file has correct naming
assert process.out.ptm_in_id_onsite[0][1].toString().endsWith('_phosphors.idXML')
assert process.out.ptm_in_id_onsite[0][1].toString().endsWith('_phosphors.idparquet')
}
}

Expand All @@ -70,7 +71,7 @@ nextflow_process {
input[0] = [
[ id: 'test', mzml_id: 'test_sample' ],
file(params.test_data['proteomics']['onsite']['mzml'], checkIfExists: true),
file(params.test_data['proteomics']['onsite']['idxml'], checkIfExists: true)
file(params.test_data['proteomics']['onsite']['idparquet'], checkIfExists: true)
]
"""
}
Expand All @@ -85,7 +86,7 @@ nextflow_process {
assert process.out.ptm_in_id_onsite.size() == 1
assert process.out.log.size() == 1
// Check output file has correct naming
assert process.out.ptm_in_id_onsite[0][1].toString().endsWith('_lucxor.idXML')
assert process.out.ptm_in_id_onsite[0][1].toString().endsWith('_lucxor.idparquet')
}
}

Expand All @@ -99,7 +100,7 @@ nextflow_process {
input[0] = [
[ id: 'test', mzml_id: 'test_sample' ],
file(params.test_data['proteomics']['onsite']['mzml'], checkIfExists: true),
file(params.test_data['proteomics']['onsite']['idxml'], checkIfExists: true)
file(params.test_data['proteomics']['onsite']['idparquet'], checkIfExists: true)
]
"""
}
Expand Down
4 changes: 2 additions & 2 deletions tests/config/test_data.config
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ params {
}
onsite {
// Test data for onsite PTM localization module
mzml = "https://ftp.pride.ebi.ac.uk/pub/databases/pride/resources/proteomes/quantms-ci-github/onsite/1.mzML"
idxml = "https://ftp.pride.ebi.ac.uk/pub/databases/pride/resources/proteomes/quantms-ci-github/onsite/1_consensus_fdr_filter_pep.idXML"
mzml = "https://ftp.pride.ebi.ac.uk/pub/databases/pride/resources/proteomes/quantms-ci-github/onsite/Phospho_redissolve_final_01.mzML"
idparquet = System.getenv('ONSITE_IDPARQUET_PATH') ?: "https://ftp.pride.ebi.ac.uk/pub/databases/pride/resources/proteomes/quantms-ci-github/onsite/Phospho_redissolve_final_01_clean_perc.idparquet"
}
qpx {
// DIA-NN results for QPX export testing (reuses pmultiqc DIANN example)
Expand Down
Loading