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

Add tests for the Dataflow Mark Duplicates by running the existing Mark Duplicates tests on the Dataflow code. #798

Merged
merged 1 commit into from
Sep 4, 2015

Conversation

tovanadler
Copy link
Contributor

No description provided.

output = new File(outputDir, "output.sam");
addArg("--INPUT", input.getAbsolutePath());
addArg("--OUTPUT", output.getAbsolutePath());
if (useStandardInputNames) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't like that we have to have this switch here. Could you open a ticket to update tools so that it is unnecessary? Subtask of #133

Copy link
Member

Choose a reason for hiding this comment

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

I just made it. #805.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Seemed like the least invasive way for now, but it's definitely a hack.

@lbergelson
Copy link
Member

@tovanadler Review complete. Lets only disable the tests for dataflow. Thanks for including the test failure reason in there.

@lbergelson lbergelson assigned tovanadler and unassigned lbergelson Aug 12, 2015
@tovanadler tovanadler assigned lbergelson and unassigned tovanadler Aug 12, 2015
@tovanadler
Copy link
Contributor Author

Thanks for the review. Back at you Louis.

@lbergelson
Copy link
Member

👍 Looks good.

Squash these down to a single commit and you're ready to merge.

@lbergelson lbergelson assigned tovanadler and unassigned lbergelson Aug 12, 2015
@tovanadler tovanadler force-pushed the tn_mark_duplicates branch 2 times, most recently from a591549 to 82699ce Compare August 12, 2015 21:35
@akiezun
Copy link
Contributor

akiezun commented Sep 4, 2015

@jean-philippe-martin can you take over? If this done? (it seems to so)

@jean-philippe-martin
Copy link
Contributor

@akiezun I think you're asking me to squash/merge this code. Is this right?

@akiezun
Copy link
Contributor

akiezun commented Sep 4, 2015

yes, thanks. rebase+squash and merge

…tests that don't pass yet, and we will revisit them to figure out where the bug is.
@jean-philippe-martin
Copy link
Contributor

rebase+squash. Merge pending green tests.

jean-philippe-martin added a commit that referenced this pull request Sep 4, 2015
Add tests for the Dataflow Mark Duplicates by running the existing Mark Duplicates tests on the Dataflow code.
@jean-philippe-martin jean-philippe-martin merged commit c178069 into master Sep 4, 2015
@jean-philippe-martin jean-philippe-martin deleted the tn_mark_duplicates branch September 4, 2015 22:25
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