Snapshot content of version files instead of a hash #6505

edmundmiller · 2024-08-30T13:17:36Z

Currently we're snapshoting the versions.yml and getting a hash:

modules/modules/nf-core/ale/tests/main.nf.test.snap

Lines 26 to 28 in 17b8bed

    
           "versions": [ 
        
               "versions.yml:md5,949da9c6297b613b50e24c421576f3f1" 
        
           ]

                "versions": [
-                    "versions.yml:md5,949da9c6297b613b50e24c421576f3f1"
+                    "content": ["{ALE={ale=20180904}}"],
                ]

edmundmiller · 2024-08-30T13:22:59Z

@nvnieuwk Pointed out that topic channels won't be hashable

#4517

famosab · 2024-09-03T14:00:54Z

How exactly would we assert for the versions then? I tried something like this:

{ assert snapshot(
             process.out,
             file(process.out.versions[0]).readLines()
             ).match()
}

We might need to adapt the linting or snapshot both md5 and the content. Otherwise this error occurs:

╭─ [✗] 1 Module Test Failed ───────────────────────────────────────────────────╮
│              ╷                               ╷                               │
│ Module name  │ File path                     │ Test message                  │
│╶─────────────┼───────────────────────────────┼──────────────────────────────╴│
│ wgsim        │ modules/nf-core/wgsim/tests/… │ versions not found in         │
│              │                               │ snapshot file                 │
│              ╵                               ╵                               │
╰──────────────────────────────────────────────────────────────────────────────╯
╭───────────────────────╮
│ LINT RESULTS SUMMARY  │
├───────────────────────┤
│ [✔]  50 Tests Passed  │
│ [!]   0 Test Warnings │
│ [✗]   1 Test Failed   │
╰───────────────────────╯
Error: Process completed with exit code 1.

Of course only if we do not assert process.out completely

{ assert snapshot(
             process.out.fastq[0][1].collect { file(it).name },
             file(process.out.versions[0]).readLines(),
             ).match()
}

Maybe there is a more elegant way?

edmundmiller · 2024-09-03T16:33:58Z

We might need to adapt the linting or snapshot both md5 and the content. Otherwise this error occurs:

Luckily, we make the rules of the linter so we can ignore any errors like that and fix it right after we settle on a standard!

I like file(process.out.versions[0]).readLines() but let me see what the snapshot looks like.

Ideally I'd love for it to look like

"sarscov2 [fasta_gz] - paired-end sorted bam": {
        "content": [
            {
                "0": [
                    [
                        {
                            "id": "test",
                            "single_end": false
                        },
                        "test_ALEoutput.txt:md5,4abcbd60ae1dbf78138c97e5fed97f3e"
                    ]
                ],
                "1": [
                    "versions.yml:md5,949da9c6297b613b50e24c421576f3f1"
                ],
                "ale": [
                    [
                        {
                            "id": "test",
                            "single_end": false
                        },
                        "test_ALEoutput.txt:md5,4abcbd60ae1dbf78138c97e5fed97f3e"
                    ]
                ],
-                "versions": [
-                    "versions.yml:md5,949da9c6297b613b50e24c421576f3f1"
-                ]
+                "versions": {
+                    "ale": "20180904"
+                }
            }
        ],
        "meta": {
            "nf-test": "0.8.4",
            "nextflow": "23.10.1"
        },
        "timestamp": "2024-03-19T09:06:19.589167"
    },

edmundmiller · 2024-09-03T16:40:31Z

Maybe there is a more elegant way?

I think we could write a lib function for this, either in this repo or the nf-test/utils plugin.

Two other things to consider:

Migration to topic channels #4517 I think we need to see how these look when snapshotted.
Renovate Workflow #4306 and the automated bumping of conda dependancies coming soon™️. I was just thinking about this and I'm proposing maybe we move to just getting the versions through the stub workflow. This would:
A) Make running them actually test something besides stub functionality
B) Allow us to do nf-test test --tag stub --update-snapshot ... in CI to automatically bump the version snapshot, and then run the actual tests without the fear of bumping any real test outputs that aren't the version. I haven't found a better way around this without requesting specific snapshot updates in nf-test itself.

SPPearce · 2024-09-05T14:06:50Z

Maybe there is a more elegant way?

I think we could write a lib function for this, either in this repo or the nf-test/utils plugin.

Two other things to consider:

Migration to topic channels #4517 I think we need to see how these look when snapshotted.

Renovate Workflow #4306 and the automated bumping of conda dependancies coming soon™️. I was just thinking about this and I'm proposing maybe we move to just getting the versions through the stub workflow. This would:
A) Make running them actually test something besides stub functionality
B) Allow us to do nf-test test --tag stub --update-snapshot ... in CI to automatically bump the version snapshot, and then run the actual tests without the fear of bumping any real test outputs that aren't the version. I haven't found a better way around this without requesting specific snapshot updates in nf-test itself.

I definitely support snapshoting the version information itself, think that is much clearer than the md5sum. The stub test could get the versions, it only needs to be once, that sounds reasonable. And every module should have a stub and stub test (eventually), so sounds good to me.

edmundmiller · 2024-09-10T16:36:34Z

There's a function for yaml files in nf-test(that may have snuck in with 0.9.0) https://www.nf-test.com/docs/assertions/files/

edmundmiller · 2024-09-10T17:02:12Z

Magic line is:

{ assert snapshot(path(process.out.versions.get(0)).yaml).match("versions") },

askimed/nf-test#258

#6505

* build: Add wave * build: Set strategy to dockerfile, conda then container * refactor: Remove container * build: Add a repo to push to * ci(wave): Add wave build https://github.com/nodejs/docker-node/blob/3c4fa6daf06a4786d202f2f610351837806a0380/.github/workflows/build-test.yml#L29 * ci(wave): Switch to all_changed_files * ci(wave): Only look for envronment.ymls * dummy: Change env * ci(wave): Remove raw format * ci(wave): Try a bunch of different things at once * ci(wave): Remove redundant fromJson and wrap in an array * ci(wave): I have no idea what I'm doing * ci(wave): Wrap it * ci(wave): Found an example https://github.com/tj-actions/changed-files/blob/main/.github/workflows/matrix-test.yml * ci(wave): Maybe quotes? * ci(wave): That'll do it * ci(wave): Fix wave install * ci(wave): Hard code an image * ci(wave): Add secrets * feat: Try a different files structure * ci(wave): First stab at building singularity images * fixup! feat: Try a different files structure * ci(wave): Add profile to matrix * ci(wave): Give up on fancy substitution * ci(wave): Add await Co-authored-by: ewels <[email protected]> * ci(wave): Switch to quay * test(wave): Add freeze and update build repo * refactor(wave): What happens if I add a container? * refactor(wave): Have both bowtie modules use the same env For the sake of demonstration * test: Cut out using wave on tests * refactor: What happens if we use the singularity one? * refactor: Keep container directives for offline download seqeralabs/wave#323 * feat: Try new singularity OCI setting nextflow-io/nextflow@f5362a7 * build: Update container name Guess #4327 broke that * chore: Bump wave-cli version * ci: Install runc * ci: Switch to singularityhub action nextflow-io/nextflow#4543 * ci: Install new singularity manually Why that action trys to build from source, idk. * ci: Install dependancies for singularity * ci: runc => crun * ci: Fix cgroup error https://blog.misharov.pro/2021-05-16/systemd-github-actions * ci: That'll do it * ci: Remove Dockerfile We'll have a seperate action for this I think * ci: Update name * ci: Push to the correct repos * ci: Remove OCI stuff * ci: Need a full URL * ci: Fix // in container name * ci: Remove push Build once, renovate should bump the images automagically * build: Add containers back * ci: Add cache repos Idk what this does exactly * ci: Change registry name to use _ Because "build" is a api end point on quay.io. So `bowtie/build` doesn't work. Other plus is this matches the conda env name. * build: / => _ in container name * Try ociAutoPull * chore: Add renovate comments to samtools Just to trigger wave build * test: Add ociAutoPull to nf-test * ci: Bump wave version * chore: Bump containers with new wave version Not sure why that's happening... * build: Update to use commity.wave.seqera.io * ci: Bump wave-cli to 1.4.1 * ci: Try apptainer * ci: Remove build-repo to see what happens * build: Bump Nextflow version requirement * fix: Get rid of the environment name? Maybe this will get the auto generated tag? * ci: Bump action versions * ci: Try name-strategy tagPrefix seqeralabs/wave-cli@269df0e * ci: Remove singularity build for now * ci: Try imageSuffix * ci: Try none * ci: What is the bowtie container name * ci: Remove --name-strategy * style: Add back in container elvis operator * ci: Remove cache repo * Revert "build: Bump Nextflow version requirement" This reverts commit 69e1ea5. * Revert "test: Add ociAutoPull to nf-test" This reverts commit 5a3d546. * test(#6505): Snapshot the versions contents, not the hash * ci(#6505): Update version snapshot after building containers * test(#6505): Attempt a topic channel with tests askimed/nf-test#258 * chore: Bump to 1.5.0 * fix: Remove shard and filter on test bumping * build: Bump images to match environment * ci: Fix nf-test setup * ci: Remove snapshot bumping * build: Fix containers in bowtie --------- Co-authored-by: ewels <[email protected]>

* ci: Attempt to split everything out Don't hate me Adam * ci: Add changed since, sharding, and ci * ci: Add filter to try to get jobs split up * ci: Switch to only-changed * ci: See if follow-dependencies works without "related-tests" * ci: Remove skipped tests Idk what we're going to do about these... * ci: Actually use the nf-test version * ci: module => process * ci: Clean up job names * dummy: Make a change * ci: Skip test.tap * ci: Add fetch-depth * ci: Clean up name * ci: Lint everything * ci: Get the job names clean * ci: Add hide-progress on linting * ci: Add psuedocode for conda-fail.yml https://nfcore.slack.com/archives/CJRH30T6V/p1724406283145319 * test: Snapshot the versions contents, not the hash #6505 * ci: Keep running nf-core lint the way it was Blocked by nf-core/tools#3140 * ci: Move conda skips out * ci: Address a comment * style: Move prettier and editorconfig to pre-commit #4554 (comment) * ci: Add note about nf-core lint pre-commit * chore: Copy over conda skips a004c86 2da71b7 * ci: only-changed => changed-since * Add confirm-pass * ci: dynamically set shards * ci: Run 3 process jobs per CI run as an example * install nf-test * fix install nf-test * fix install nf-test * ci: Remove dynamic number of shards, and combine process and workflow filters * ci: Remove variable Nextflow versions * test: Update bowtie versions * ci: Fix indention * ci: We're not testing multiple python versions * style: Clean up job names * build: Remove invisible characters? * test(bowtie): Bump snapshots * style: Remove stray comment * ci: Add GPU tests in their own workflows These will only get trigger on the paths listed * ci(gpu): Clean up triggers and make a note about how to make the includes smarter * ci: pip install cryptography * ci: Try removing path * ci: Add fetch-depth and remove duplicate checkout * test: Remove the second gpu profile * ci: Add path back in * ci(gpu): Remove changed-since * ci: Fix missed dorny/paths-filter update Co-authored-by: mashehu <[email protected]> * Add changes from codereview Co-authored-by: mashehu <[email protected]> * style: Remove example TODO comments https://github.com/nf-core/modules/pull/6286/files#r1846856369 Co-authored-by: mashehu <[email protected]> * refactor: Try tags for GPU CI askimed/nf-test#272 * style: Set NFT_WORKDIR as an ENV variable * style: Run lsp format on a parabricks module For science * test: Add gpu tags to parabricks * ci: Split tags out of matrix * Revert "style: Run lsp format on a parabricks module" This reverts commit d134e78. * ci: Fix name --------- Co-authored-by: Edmund Miller <[email protected]> Co-authored-by: maxulysse <[email protected]> Co-authored-by: mashehu <[email protected]>

edmundmiller · 2024-12-06T04:54:54Z

askimed/nf-test#197

* test(#6505): Add pigz to versions.yml * feat: Add pigz to trimgalore nf-core/atacseq#394 FelixKrueger/TrimGalore#16 (comment) https://github.com/FelixKrueger/TrimGalore/blob/master/CHANGELOG.md#version-060-release-on-1-mar-2019 * style: Run lsp format * Update modules/nf-core/trimgalore/main.nf --------- Co-authored-by: Simon Pearce <[email protected]>

edmundmiller mentioned this issue Aug 30, 2024

Batch module updates - Fall 2024 #5828

Open

edmundmiller assigned maxulysse Aug 30, 2024

edmundmiller added a commit that referenced this issue Sep 10, 2024

test(#6505): Snapshot the versions contents, not the hash

daa44fe

edmundmiller added a commit that referenced this issue Sep 10, 2024

ci(#6505): Update version snapshot after building containers

457bf9e

edmundmiller added a commit that referenced this issue Sep 10, 2024

test(#6505): Attempt a topic channel with tests

2925c17

askimed/nf-test#258

edmundmiller mentioned this issue Sep 12, 2024

style: Make version snapshots pretty nf-core/nascent#163

Merged

edmundmiller added a commit that referenced this issue Sep 18, 2024

test: Snapshot the versions contents, not the hash

5594071

#6505

edmundmiller added a commit that referenced this issue Sep 24, 2024

test: Snapshot the versions contents, not the hash

4806fdd

#6505

edmundmiller added a commit that referenced this issue Sep 27, 2024

chore(#6505): Capture actual version yaml instead of hash

1fe2e6d

edmundmiller added a commit that referenced this issue Sep 30, 2024

test: Snapshot the versions contents, not the hash

2a218e3

#6505

This was referenced Nov 26, 2024

Check for new version snapshotting #7098

Draft

spaceranger_bump_version #7088

Merged

edmundmiller added a commit that referenced this issue Dec 6, 2024

test(#6505): Add pigz to versions.yml

8e2a01f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snapshot content of version files instead of a hash #6505

Snapshot content of version files instead of a hash #6505

edmundmiller commented Aug 30, 2024 •

edited

Loading

edmundmiller commented Aug 30, 2024

famosab commented Sep 3, 2024 •

edited

Loading

edmundmiller commented Sep 3, 2024

edmundmiller commented Sep 3, 2024

SPPearce commented Sep 5, 2024

edmundmiller commented Sep 10, 2024

edmundmiller commented Sep 10, 2024

edmundmiller commented Dec 6, 2024

Snapshot content of version files instead of a hash #6505

Snapshot content of version files instead of a hash #6505

Comments

edmundmiller commented Aug 30, 2024 • edited Loading

edmundmiller commented Aug 30, 2024

famosab commented Sep 3, 2024 • edited Loading

edmundmiller commented Sep 3, 2024

edmundmiller commented Sep 3, 2024

SPPearce commented Sep 5, 2024

edmundmiller commented Sep 10, 2024

edmundmiller commented Sep 10, 2024

edmundmiller commented Dec 6, 2024

edmundmiller commented Aug 30, 2024 •

edited

Loading

famosab commented Sep 3, 2024 •

edited

Loading