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

Check *.txt files for integration tests #181

Closed
wants to merge 5 commits into from

Conversation

Colelyman
Copy link
Contributor

When debugging the find_indels_substitution function, I noticed that these files didn't make it over from the other repo during the migration. In any case, these changes now diff across all of the *.txt files in the integration tests.

I have also updated the CircleCI script to use this as well, there may be bugs in that update because I haven't been able to test it.

There is also a small bug fix included here for CRISPRessoCompare where the sample names weren't being properly loaded.

This script will diff entire directories and filter for *.txt files. It will
also round floating point numbers to account for the potential discrepency in
floating point representation.
This was a place where it was (partially) missed during the crispresso2_info
object refactoring.
@Colelyman Colelyman mentioned this pull request Dec 10, 2021
@kclem
Copy link
Member

kclem commented Dec 11, 2021

Hey Cole, thanks, this is great.

I had considered including all files for the test comparisons, but was hesitant because it may bloat the checkout or release size (I suppose we could tag this dir with export-ignore). Instead, I chose a few summary files which should show changes in the case of algorithmic differences.

We probably don't need the pdfs or pngs, though right?

What are your thoughts?

@Colelyman
Copy link
Contributor Author

Hey Kendell,

I have been thinking about this, and I wonder if a good solution is to keep all of these solutions in a separate repo, and add then as a git submodule? That way the large folders won't be pulled by default, but those that want to run them can easily pull them in.

As for the pdfs and pngs, I was looking into this SO question and was thinking about comparing the pngs and measuring the difference between the images. I thought I would implement this feature later, but I think we could remove the image files for now and add them later when (if) this gets implemented.

@Colelyman Colelyman closed this Jan 11, 2022
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.

None yet

2 participants