Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test multiqc #439

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Test multiqc #439

wants to merge 20 commits into from

Conversation

michael-harper
Copy link
Contributor

No description provided.

Copy link
Contributor

@vivbak vivbak left a comment

Choose a reason for hiding this comment

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

Good job!

test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
test/jobs/test_multiqc.py Show resolved Hide resolved
test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
Copy link
Contributor

@daniaki daniaki left a comment

Choose a reason for hiding this comment

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

Nice work, you have a really clean testing style :) Added some suggestions about restructuring a few tests, advice on mock usage and some other minor nitpicks.

test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
test/jobs/test_multiqc.py Show resolved Hide resolved
test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
test/jobs/test_multiqc.py Show resolved Hide resolved
test/jobs/test_multiqc.py Show resolved Hide resolved
test/jobs/test_multiqc.py Outdated Show resolved Hide resolved
Copy link
Contributor

@KatalinaBobowik KatalinaBobowik left a comment

Choose a reason for hiding this comment

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

👍

@michael-harper michael-harper requested review from daniaki and vivbak and removed request for katiedelange September 27, 2023 06:25
Copy link
Contributor

@daniaki daniaki left a comment

Choose a reason for hiding this comment

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

LGTM!

@michael-harper michael-harper requested review from vivbak and removed request for vivbak September 27, 2023 06:47
@michael-harper michael-harper requested review from vivbak and removed request for vivbak September 27, 2023 06:48
michael-harper and others added 3 commits September 27, 2023 16:49
…months ago (PR here: #533) where multiqc output was being overwritten by subsequent batch runs and so it was changed to be the hash of input files as prefix to filenames so that each multiqc report is specific to a batch being run and separate batches dont overwrite previous multiqc files.
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.

4 participants