Skip to content

samplesheetparser: add two identically-named non-colliding V1 sheets for merge collision test - #2238

Closed
chaitanyakasaraneni wants to merge 2 commits into
nf-core:modulesfrom
chaitanyakasaraneni:add-samplesheetparser-testdata
Closed

samplesheetparser: add two identically-named non-colliding V1 sheets for merge collision test#2238
chaitanyakasaraneni wants to merge 2 commits into
nf-core:modulesfrom
chaitanyakasaraneni:add-samplesheetparser-testdata

Conversation

@chaitanyakasaraneni

Copy link
Copy Markdown

Why two identically-named sheets

These fixtures were added to address review feedback on nf-core/modules#12774, where a reviewer asked for a samplesheetparser/merge test covering two identically-named input sheets.

The samplesheetparser/merge module takes a collection of sample sheets and merges them. In real use, per-project sheets are almost always all named SampleSheet.csv, and Nextflow raises an "input file name collision" error when two staged files share a basename. The module handles this with stageAs: "input*/*" (same pattern as samplesheetparser/diff).

To actually test that identically-named inputs are staged and merged correctly, the nf-test needs two files that share the basename SampleSheet.csv but live in separate directories (a single flat directory can't hold two files of the same name). These fixtures provide exactly that:

  • bcl/samesheetparser/a/SampleSheet.csv
  • bcl/samesheetparser/b/SampleSheet.csv

They are copies of the existing non-colliding V1 sheets (flowcell_samplesheet.v1.csv and flowcell_samplesheet_b.v1.csv), so the merge succeeds and the test can assert a clean result rather than a failure path. Without same-named fixtures, the collision case can only be exercised by merging a sheet with itself, which trips the index-collision check and exits non-zero.

@chaitanyakasaraneni
chaitanyakasaraneni force-pushed the add-samplesheetparser-testdata branch from 7f20fa3 to e303f37 Compare August 26, 2026 02:37

@erikrikarddaniel erikrikarddaniel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 But there's a prek error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants